Closed
Bug 1157752
Opened 9 years ago
Closed 9 years ago
vertical text with upright orientation should force LTR directionality
Categories
(Core :: Layout: Text and Fonts, defect)
Core
Layout: Text and Fonts
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: jfkthame, Assigned: jfkthame)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 2 obsolete files)
6.17 KB,
patch
|
Details | Diff | Splinter Review | |
3.62 KB,
patch
|
smontagu
:
review+
|
Details | Diff | Splinter Review |
See the Writing Modes spec [1]: # In vertical writing modes, characters from horizontal-only scripts are # rendered upright, i.e. in their standard horizontal orientation. Characters # from vertical scripts are set with their intrinsic orientation and shaped # normally. See Vertical Orientations for further details. # # For the purposes of bidi reordering, this value causes all characters to be # treated as strong LTR. This value causes the used value of direction to be ltr. So when Hebrew or Arabic characters (for example) occur in vertical text with upright orientation, they should flow *down* the page just like Latin characters. [1] http://dev.w3.org/csswg/css-writing-modes/#valdef-text-orientation-upright
Assignee | ||
Comment 1•9 years ago
|
||
One idea I had here is to make nsBidiPresUtils recognize the combination of vertical writing mode and upright orientation and process it in the same way as it already handles unicode-bidi:bidi-override. This behaves as expected for the (fairly simple) things I've tested so far; does it seem like a reasonable approach?
Attachment #8596634 -
Flags: review?(smontagu)
Assignee | ||
Updated•9 years ago
|
Attachment #8596634 -
Attachment description: In vertical writing modes, text-orientation:vertical should force LTR directionality → In vertical writing modes, text-orientation:upright should force LTR directionality
Comment 2•9 years ago
|
||
I would have suggested doing it this way myself, but let me play devil's advocate for a minute: what about a <bdo dir="rtl"> inside an element with text-orientation:upright? It looks as if that will just be ignored, are we OK with that?
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Simon Montagu :smontagu from comment #2) > I would have suggested doing it this way myself, but let me play devil's > advocate for a minute: what about a <bdo dir="rtl"> inside an element with > text-orientation:upright? It looks as if that will just be ignored, are we > OK with that? I think so. I wondered about this for a while, but ISTM this is the most reasonable position. I'd consider "writing-mode:vertical-{lr,rl} + text-orientation:upright" to be a "higher-level protocol" that overrides the Bidi_Class of all the characters for the purposes of the bidi algorithm. While <bdo dir="rtl"> also overrides Bidi_Class, it's a low-level control that is very close to the raw character stream (equivalent to using the Unicode RIGHT-TO-LEFT-OVERRIDE character), while writing-mode/text-orientation is higher level styling; and the higher-level protocol overrides the characters-stream-level properties. If we allow bidi overrides to operate within vertical/upright content, where all text is normally forced to behave as LTR, then we get the odd situation whereby english text HEBREW TEXT english text and english text <bdo dir=rtl>HEBREW TEXT</bdo> english text are indistinguishable in horizontal writing mode, or in vertical/sideways or vertical/mixed, yet in vertical/upright they suddenly mean different things. As it is, we have distinctions that get lost in vertical/upright (in other modes, "<bdo dir=rtl>abc</bdo>" renders differently from plain "abc", but in vertical/upright there's no difference because <bdo> is overridden). But I think it's much more reasonable to say that vertical/upright eliminates all directionality distinctions (just like it erases distinctions between the inherent Bidi_Class of Unicode characters) than to say that it causes distinctions to appear that would not exist in any other mode.
Assignee | ||
Comment 4•9 years ago
|
||
Incidentally, I was going to check webkit/blink behavior on this issue, but neither Safari nor Chrome appears to support the vertical/upright-forces-LTR feature described by the spec, so they're not useful for comparison. (I think it's quite clear, though, that what the spec describes is the right thing to do for vertical/upright rendering, and this is simply a current limitation of those implementations.)
Comment 5•9 years ago
|
||
Comment on attachment 8596634 [details] [diff] [review] In vertical writing modes, text-orientation:upright should force LTR directionality Review of attachment 8596634 [details] [diff] [review]: ----------------------------------------------------------------- r=me. I think it would probably have been cleaner to pass the style context to PushBidiControl and do everything in there, rather than in a separate method. Right now that would be hard to do because of the isRTL check in BidiParagraphData::Reset, so I'm not suggesting you change it now, but that is a hack which I expect to remove after we have proper support for bidi isolates, and then we can tighten this up in due course. ::: layout/base/nsBidiPresUtils.cpp @@ +63,5 @@ > +// unicode-bidi:bidi-override, or text-orientation:upright in vertical writing > +// mode) when applying the bidi algorithm. > +// If aHandleEmbed is true, also handles unicode-bidi:embed. > +// Returns 0 if no directional control character is implied by the style. > +static char16_t GetBidiOverride(nsStyleContext* aStyleContext, Nit: maybe call this GetBidiControl to include the embed controls (and looking ahead to the isolate controls that we will need to add in or following from bug 1157726), and adjust the comment above to correspond.
Assignee | ||
Updated•9 years ago
|
Attachment #8596634 -
Attachment is obsolete: true
Attachment #8596634 -
Flags: review?(smontagu)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Assignee | ||
Comment 7•9 years ago
|
||
Here's a simple reftest to go with it.
Attachment #8597928 -
Flags: review?(smontagu)
Comment 8•9 years ago
|
||
Comment on attachment 8597928 [details] [diff] [review] Reftest for vertical mode with upright orientation overriding bidi directionality Review of attachment 8597928 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/reftests/writing-mode/1157752-upright-bidi-ref.html @@ +31,5 @@ > +Bidi עברית text > +</div> > + > +<div class=upright> > +<rlo dir=ltr>Bidi עברית text</rlo> Shouldn't this be <bdo dir=ltr> here and 4 lines below? And that makes me wonder whether the test is really testing anything. Does it in fact fail without the patch?
Assignee | ||
Comment 9•9 years ago
|
||
(In reply to Simon Montagu :smontagu from comment #8) > Comment on attachment 8597928 [details] [diff] [review] > Reftest for vertical mode with upright orientation overriding bidi > directionality > > Review of attachment 8597928 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: layout/reftests/writing-mode/1157752-upright-bidi-ref.html > @@ +31,5 @@ > > +Bidi עברית text > > +</div> > > + > > +<div class=upright> > > +<rlo dir=ltr>Bidi עברית text</rlo> > > Shouldn't this be <bdo dir=ltr> here and 4 lines below? Duh -- yes, of course it should. I blame lack of coffee, or something like that. I'd originally written it using Unicode character entities, and decided at the last minute to change to the html tags for better readability. Except that I did it wrong. :( > And that makes me > wonder whether the test is really testing anything. Does it in fact fail > without the patch? It does, actually, though more by luck than anything else. :) The incorrect tag ends up as an inline element with LTR directionality within the RTL div, and that affects the ordering of the hebrew and english runs within it. But anyway, I'll fix it to actually say what I intended -- thanks.
Assignee | ||
Comment 10•9 years ago
|
||
Reftest, corrected.
Attachment #8598464 -
Flags: review?(smontagu)
Assignee | ||
Updated•9 years ago
|
Attachment #8597928 -
Attachment is obsolete: true
Attachment #8597928 -
Flags: review?(smontagu)
Updated•9 years ago
|
Attachment #8598464 -
Flags: review?(smontagu) → review+
Assignee | ||
Comment 11•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3775b767e155 https://hg.mozilla.org/integration/mozilla-inbound/rev/169b3c5fc41c
https://hg.mozilla.org/mozilla-central/rev/3775b767e155 https://hg.mozilla.org/mozilla-central/rev/169b3c5fc41c
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in
before you can comment on or make changes to this bug.
Description
•