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)

defect
Not set
normal

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)

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
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)
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
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?
(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.
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 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.
Updated as suggested in comment 5; carrying over r=smontagu.
Attachment #8596634 - Attachment is obsolete: true
Attachment #8596634 - Flags: review?(smontagu)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Here's a simple reftest to go with it.
Attachment #8597928 - Flags: review?(smontagu)
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?
(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.
Attachment #8597928 - Attachment is obsolete: true
Attachment #8597928 - Flags: review?(smontagu)
Attachment #8598464 - Flags: review?(smontagu) → review+
You need to log in before you can comment on or make changes to this bug.