[Webkit-unassigned] [Bug 50949] Add support for unicode-bidi:plaintext CSS property

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=50949





--- Comment #14 from Eric Seidel <eric at webkit.org>  2011-06-21 17:55:56 PST ---
(From update of attachment 98088)
View in context: https://bugs.webkit.org/attachment.cgi?id=98088&action=review

I think I need to look at this again later.

> Source/WebCore/html/HTMLElement.cpp:129
> +static int unicodeBidiAttributeForDirAuto(HTMLElement* element)

Should this be marked inline?

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:115
> +static void determineParagraphDirection(TextDirection& dir, InlineIterator iter)

Should InlineIterator be passed as a reference?  do you actually want it to be copied here?  Maybe that would be more clear with a const & which you explicitly copy?

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:967
> +        bool isNewParagraph = lineInfo.previousLineBrokeCleanly();

is this true?  that it's a "new paragraph"?  You might want to say isNewUBAParagrah?  I'm not sure.  Do we need a new accessor on lineInfo which talks about UBA things instead of previous line?  They may be backed by the same bool, but I wonder if we shouldn't have a UBA-specific accessor on lineInfo, since it's not clear from previousLineBrokeCleanly what it applies to...

> Source/WebCore/rendering/style/RenderStyle.h:261
> -        // 50 bits
> +        // 51 bits

Did you validate this count?  Sometimes these comments are out of date.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.