[webkit-reviews] review denied: [Bug 69707] Shrink RootInlineBox. : [Attachment 110280] Proposed patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Oct 9 19:33:07 PDT 2011


Darin Adler <darin at apple.com> has denied Andreas Kling <kling at webkit.org>'s
request for review:
Bug 69707: Shrink RootInlineBox.
https://bugs.webkit.org/show_bug.cgi?id=69707

Attachment 110280: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=110280&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=110280&action=review


> Source/WebCore/rendering/InlineFlowBox.h:302
> +protected:

I don’t think you should restate “protected”. It’s OK but a little strange,
since we don’t normally repeat these.

> Source/WebCore/rendering/InlineFlowBox.h:314
> +    WTF::Unicode::Direction m_lineBreakBidiStatusEor : 5;
> +    WTF::Unicode::Direction m_lineBreakBidiStatusLastStrong : 5;
> +    WTF::Unicode::Direction m_lineBreakBidiStatusLast : 5;

These need to be unsigned, not Direction, since Direction is an enum. It’s the
same MSVC/enum/bitfield problem we have elsewhere. Also, probably don’t need
the WTF:: prefix here because of the strange way WTF uses its namespace.

I know you just moved these members, but they will malfunction if the direction
is a value with the high bit set. If the code worked OK on Windows before
there’s a chance we could get away with 4-bit bitfields.


More information about the webkit-reviews mailing list