[Webkit-unassigned] [Bug 52128] ISO-8859-8 Hebrew text displayed reversed with dir="rtl"

[bugzilla-daemon at webkit.org]

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


mitz at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #82015|review?                     |review+
               Flag|                            |




--- Comment #20 from mitz at webkit.org  2011-02-13 13:10:13 PST ---
(From update of attachment 82015)
View in context: https://bugs.webkit.org/attachment.cgi?id=82015&action=review

> Source/WebCore/ChangeLog:11
> +        (WebCore::::createBidiRunsForLine): For lines with visually direction override, create bidi runs without resolving bidi levels (one run per render object), set bidi level as 0 or 1 depends on LTR or RTL override, and reverse runs for RTL override.

You may want to use shorter lines in the change log (~100 characters seems to be the norm). Also change “visually” to “visual” and “depends” to “depending”.

> LayoutTests/fast/text/international/iso-8859-8.html:4
> +<meta http-equiv="Content-Type" content="text/html; charset=ISO-8859-8"/>

The / before the > is not necessary. And in HTML5 this can be expressed simply as <meta charset="ISO-8859-8">.

> LayoutTests/fast/text/international/iso-8859-8.html:13
> +<div contenteditable class="test">ùðá</div>
> +<div contenteditable class="test">ùðá â÷ë </div>
> +<div contenteditable class="test">ùðá abc â÷ë</div>
> +<div contenteditable class="test">abc ùðá def</div>
> +<div contenteditable class="test">ùðá <span dir=ltr>â÷ë</span></div>
> +<div contenteditable class="test">ùðá <span dir=rtl>â÷ë</span></div>

Do these really need to be contenteditable?

> LayoutTests/fast/text/international/iso-8859-8.html:24
> +    function log(str)
> +    {
> +        var li = document.createElement("li");
> +        li.appendChild(document.createTextNode(str));
> +        var console = document.getElementById("console");
> +        console.appendChild(li);
> +    }

Since you’re already including js-test-pre.js, why not use testPassed() and testFailed()?

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