[webkit-reviews] review granted: [Bug 121221] RenderBR should not be RenderText : [Attachment 211899] with shouted OVERRIDE for qt bots
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Sep 17 07:55:15 PDT 2013
Darin Adler <darin at apple.com> has granted Antti Koivisto <koivisto at iki.fi>'s
request for review:
Bug 121221: RenderBR should not be RenderText
https://bugs.webkit.org/show_bug.cgi?id=121221
Attachment 211899: with shouted OVERRIDE for qt bots
https://bugs.webkit.org/attachment.cgi?id=211899&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=211899&action=review
Sorry, I later figured out I was wrong about OVERRIDE, but forgot to tell you.
> Source/WebCore/dom/Range.cpp:1651
> + if (r->isBR()) {
> + r->absoluteQuads(quads, &isFixed);
> + } else if (r->isText()) {
Normally our code style says to omit the braces in a case like this.
> Source/WebCore/rendering/RenderBR.cpp:3
> + * Copyright (C) 2006, 2013 Apple Computer, Inc.
Should be Apple Inc. Should say All rights reserved.
> Source/WebCore/rendering/RenderBR.cpp:192
> + IntRect boundingBox = linesBoundingBox();
> + return IntRect(0, 0, boundingBox.width(), boundingBox.height());
I think you can do it like one of these two:
return IntRect(IntPoint(), linesBoundingBox().size();
return IntRect(IntPoint(0, 0), linesBoundingBox().size();
I like those slightly better than the two line form.
> LayoutTests/ChangeLog:16
> + Changes in render tree dumb that don't affect rendering.
dump, not dumb
> LayoutTests/ChangeLog:25
> + Changes in render tree dumb that don't affect rendering.
Ditto.
More information about the webkit-reviews
mailing list