[Webkit-unassigned] [Bug 50916] Add support for dir=auto

[bugzilla-daemon at webkit.org]

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





--- Comment #19 from Yael <yael.aharon at nokia.com>  2011-02-03 07:03:46 PST ---
(In reply to comment #17)
Thanks for your comments,
> (From update of attachment 80968 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=80968&action=review
> > Source/WebCore/html/HTMLElement.cpp:868
> > +    ASSERT(equalIgnoringCase(fastGetAttribute(dirAttr), "auto"));
> 
> It's only legal to call this method on an element which has dir=auto set?
> Could you add a comment on why that is?
> 
I am not sure what is the use for calling this function unless the element has the attribute dir="auto" ?

> > Source/WebCore/html/HTMLElement.cpp:895
> > +    return direction;
> 
> It looks like you don't modify direction in the function body, can you just return LTR here?
> 
True, direction is redundant here and can be removed.

> > LayoutTests/fast/dom/HTMLElement/attr-dir-auto-change-child-node.html:19
> > +description('Test that when an element has dir attribute with value "auto", if its first child changes, the element\'s directionality is re-evaluated.');
> 
> How about:
> Test that directionality is re-evaluated when an element with dir=auto set, has it's first child modified.
> 
ok

> The current description was a little hard for me to parse :) , same comment for other tests below.
> 
> > LayoutTests/fast/dom/HTMLElement/attr-dir-auto.html:17
> > +<div id="ltr" dir="auto" class="testDiv">Test test test</div>
> 
> Perhaps give this an id of left-to-right or similar so someone looking at the test doesn't get confused with the dir attribute?
> 
ok

> > LayoutTests/fast/dom/HTMLElement/attr-dir-auto.html:19
> > +<div id="rtl1" dir="auto" class="testDiv">מקור השם עברית</div>
> 
> How about putting an "!" at the end of the string e.g. 
> FOO!
> 
> For RTL the ! will be on the right side and for LTR it'll be on the left side.
> 
> This way you can phrase the pass/fail text above as:
> "!" should be on the right side of the string.
> 
> This makes it easier for non-RTL speakers to tell that the test is passing when doing a visual inspection.
The Hebrew text looks like garbage in the "expected results" text file, so I would not want to keep it in the expected results. Other tests that I saw also remove the text  from the expected results. 
How about if I add a comment explaining that a green border indicates right-to-left and red border indicates left-to-right ?
Of course, if there is a convention for right to left testing, I will be happy to follow that.

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