[Webkit-unassigned] [Bug 52128] ISO-8859-8 Hebrew text displayed reversed with dir="rtl"
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Feb 1 10:35:14 PST 2011
https://bugs.webkit.org/show_bug.cgi?id=52128
mitz at webkit.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #78755|review? |review-
Flag| |
--- Comment #11 from mitz at webkit.org 2011-02-01 10:35:14 PST ---
(From update of attachment 78755)
View in context: https://bugs.webkit.org/attachment.cgi?id=78755&action=review
> Source/WebCore/ChangeLog:15
> + Fix rendering of ISO-8859-8 when dir="rtl".
> + https://bugs.webkit.org/show_bug.cgi?id=52128
> +
> + Test: fast/dom/iso-8859-8.html
> +
> + * platform/text/BidiResolver.h:
> + (WebCore::::createBidiRunsForLine):
> + * rendering/RenderBlockLineLayout.cpp:
> + (WebCore::RenderBlock::constructLine):
> + (WebCore::RenderBlock::layoutInlineChildren):
> +
This should explain what is changing and why. I can’t even review the patch without knowing what it’s trying to do.
> Source/WebCore/platform/text/BidiResolver.h:965
> + } else if (direction == RTL) {
> + reverseRuns(0, runCount() - 1);
> }
There shouldn’t be braces around a single-line block.
> LayoutTests/ChangeLog:9
> + * fast/dom/iso-8859-8.html: Added.
This is not a DOM change. Please don’t put the test in fast/dom.
> LayoutTests/fast/dom/iso-8859-8.html:55
> + function checkRenderingResult(sel, node, offset, direction, element, index) {
> + for (var i = 0; i < element.innerText.length; ++i) {
> + sel.setBaseAndExtent(node, offset, node, offset);
> + if (direction == "ltr") {
> + sel.modify("move", "right", "character");
> + sel.modify("extend", "left", "character");
> + } else {
> + sel.modify("move", "left", "character");
> + sel.modify("extend", "right", "character");
> + }
> + if (direction == "ltr") {
> + assertEqual("Test " + index + " " + direction + " the " + i + "th character from left", sel.toString(), element.innerText[i]);
> + } else {
> + assertEqual("Test " + index + " " + direction + " the " + i + "th character from right", sel.toString(), element.innerText[i]);
> + }
> + node = sel.baseNode;
> + offset = sel.baseOffset;
> + }
> + }
Using the non-trivial logic for modifying selection is a very indirect way to test this change. The best test for this would be a render tree dump. Those include the directionality (and override state) of individual text runs, which should suffice. If you can’t do that, then you can test directly for the visual position of characters using Range.getClientRects().
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list