[Webkit-unassigned] [Bug 50969] <option> should support the dir attribute and be displayed accordingly both in the dropdown and after being chosen

[bugzilla-daemon at webkit.org]

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


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #81072|review?                     |review+, commit-queue-
               Flag|                            |




--- Comment #6 from David Levin <levin at chromium.org>  2011-02-04 11:29:25 PST ---
(From update of attachment 81072)
View in context: https://bugs.webkit.org/attachment.cgi?id=81072&action=review

I think you can address my comments above and commit this. Thanks!

> Source/WebCore/ChangeLog:5
> +        Implement "<option> should support the dir attribute and be displayed accordingly both in the drop-down and after being chosen" for chromium port after r76983.

This is pretty verbose.

"<option> should implement the dir attribute."

sums it up. (Just exposing the attribute isn't implementing it. The code needs to display the text correctly, etc.)

> Source/WebCore/ChangeLog:12
> +        Turn on it for <select>.

This is really long.

I think you can leave it out entirely. You explained it well in the file/function level comments.

> Source/WebCore/platform/chromium/PopupMenuChromium.cpp:84
> +    false // restrictWidthOfListBox

For future reference, misc formatting fixes mixed in a fix on lines where you aren't doing changes is frowned on because it obscures the change being done, but ok.

> Source/WebKit/chromium/ChangeLog:13
> +

Same comments as before.

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