Closed
Bug 1127488
Opened 10 years ago
Closed 10 years ago
text-align and direction in vertical writing-mode
Categories
(Core :: Layout: Block and Inline, defect)
Core
Layout: Block and Inline
Tracking
()
RESOLVED
FIXED
mozilla38
Tracking | Status | |
---|---|---|
firefox38 | --- | fixed |
People
(Reporter: bugzilla, Assigned: smontagu)
References
(Blocks 1 open bug)
Details
(Keywords: testcase, Whiteboard: Updated links to tests are in comment 14)
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
jfkthame
:
review+
|
Details | Diff | Splinter Review |
Tests
-----
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-text-align-vertical-rl-004.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-text-align-vertical-lr-005.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-text-align-vertical-rl-016.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-text-align-vertical-lr-017.xht
Expected results
----------------
A green square and no red
Explanation
-----------
The initial, default text-align given by direction declaration must be overriden by text-align declaration. Eg. 'direction: rtl; text-align: left' implies that 'text-align: left' has precedence.
Notes
-----
- I have not yet submitted those tests to the test.csswg.org repository but I will, along with correspondent reference files
- IE11, Chrome 40.0.2214.93 pass these 4 tests. Prince 9.0.5 also passes the 2 vertical-rl ones (004 and 016 tests)
- I am using Firefox 38.0a1 buildID=20150125230903
- I use Linux 3.16.0-29-generic x86_64, Qt: 4.8.6, KDE 4.14.1; Kubuntu (utopic) 14.10
- I've searched for duplicates and did not find any
Reporter | ||
Updated•10 years ago
|
Blocks: writing-mode
Keywords: testcase
Reporter | ||
Updated•10 years ago
|
OS: Linux → All
Hardware: x86_64 → All
Comment 1•10 years ago
|
||
I guess we currently do not handle RTL on vertical writing mode at all. The inline-start should be bottom if writing-mode is vertical and direction is RTL, but it seems we always treat inline-start as top in vertical text.
Reporter | ||
Comment 2•10 years ago
|
||
Xidorn Quan,
You are correct and your comment is excellent. I am quickly adding 2 additional tests based on your useful feedback:
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s62-direction-vrl-002.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s62-direction-vlr-003.xht
Gérard
Assignee | ||
Comment 3•10 years ago
|
||
I have a fix for the testcases linked here, currently testing that it doesn't regress other cases
Assignee: nobody → smontagu
Reporter | ||
Comment 4•10 years ago
|
||
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s62-direction-vrl-004.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s62-direction-vlr-005.xht
added for testing completeness purposes.
Assignee | ||
Comment 5•10 years ago
|
||
I thought it wise to test (almost) all the combinations and permutations of direction, writing-mode and text-align while we're here. text-align: justify seems like a different ball game and not relevant to this bug. I hesitated about text-align: center and eventually decided to leave it out, but I'll add it if you think it's worth it.
Attachment #8560974 -
Flags: review?(jfkthame)
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #8560975 -
Flags: review?(jfkthame)
Reporter | ||
Comment 7•10 years ago
|
||
> I thought it wise to test (almost) all the combinations and permutations of
> direction, writing-mode and text-align while we're here.
Yes, it is wise and better.
> text-align: justify
> seems like a different ball game and not relevant to this bug. I hesitated
> about text-align: center and eventually decided to leave it out, but I'll
> add it if you think it's worth it.
Huh... it is okay with me to leave out 'text-align: center'. I am not sure why you wanted my opinion (or approval? or consent?) on this here.
Anyway... 22 tests testing combinations of direction (rtl, ltr, default), text-align (left, right, center, default), vertical writing-mode (vertical-rl, vertical-lr) have been submitted to test.csswg.org :
http://lists.w3.org/Archives/Public/public-css-testsuite/2015Feb/0002.html
There ought to be a way for all sides involved here to reduce duplicate efforts when creating tests and reftests... Ideally, if the folders structure was the same at both (mozilla-central/source/layout/reftests/writing-mode/ and http://test.csswg.org/source/writing-mode/) places... Mozilla does not make use of a /support folder but uses a ../fonts/ folder.
----------
Just curious... I wonder what "fuzzy(255,402)" actually means or refer to.
Comment 8•10 years ago
|
||
Comment on attachment 8560975 [details] [diff] [review]
Patch: map correctly between abstract and physical directions of text-align and writing-mode
Review of attachment 8560975 [details] [diff] [review]:
-----------------------------------------------------------------
::: layout/generic/nsLineLayout.cpp
@@ +3018,5 @@
> + *
> + * In vertical modes, align-left and align-right map to line-left
> + * and line-right, i.e. logical inline start and end, but
> + * align-start and align-end map to inline start and end for ltr,
> + * and inline end and start for rtl.
I'm struggling to understand this, although the patch as written seems to provide the expected results.
When writing-mode is vertical and dir is RTL, inline-start should be physical bottom, and align-start should still map to inline-start, just as it does in horizontal mode. And align-left would map to line-left, which is physical top but logical inline end.
My suspicion is that we're failing to handle RTL inline dir properly for vertical mode at some other level, and that's why this patch currently results in the correct alignment: a case of two wrongs making a right. Maybe that's OK as an interim fix, but I'd like to understand this better -- or see where I'm currently misunderstanding it all -- before we press ahead.
Note that we are not doing bidi reordering properly in vertical mode; I wonder if fixing that would result in things making better sense here too? I've just filed bug 1131013 on this.
Assignee | ||
Comment 9•10 years ago
|
||
As we discussed on IRC, this uses sized inline-blocks instead of Ahem glyphs, which avoids the need for fuzzy matching.
Attachment #8560974 -
Attachment is obsolete: true
Attachment #8560974 -
Flags: review?(jfkthame)
Attachment #8561957 -
Flags: review?(jfkthame)
Assignee | ||
Comment 10•10 years ago
|
||
Comment on attachment 8560975 [details] [diff] [review]
Patch: map correctly between abstract and physical directions of text-align and writing-mode
If we take the patch for bug 1131013, we don't need this.
Attachment #8560975 -
Attachment is obsolete: true
Attachment #8560975 -
Flags: review?(jfkthame)
Comment 11•10 years ago
|
||
Comment on attachment 8561957 [details] [diff] [review]
Reftests v.2
Review of attachment 8561957 [details] [diff] [review]:
-----------------------------------------------------------------
They look reasonable but I didn't go through them all individually! Anyway, rs=me. :)
Attachment #8561957 -
Flags: review?(jfkthame) → review+
Assignee | ||
Comment 12•10 years ago
|
||
Fixed by bug 1131013
Checked in the reftests https://hg.mozilla.org/integration/mozilla-inbound/rev/c14208fc79c8.
Depends on: 1131013
Flags: in-testsuite+
Comment 13•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox38:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Reporter | ||
Comment 14•9 years ago
|
||
(In reply to Gérard Talbot from comment #0)
> Tests
> -----
>
> http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-text-align-
> vertical-rl-004.xht
>
> http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-text-align-
> vertical-lr-005.xht
>
> http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-text-align-
> vertical-rl-016.xht
>
> http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-text-align-
> vertical-lr-017.xht
These tests have been filename-renamed as following:
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-text-align-vrl-004.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-text-align-vlr-005.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-text-align-vrl-016.xht
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s71-text-align-vlr-017.xht
and those 4 tests along with 14 other tests have been submitted to the test.csswg.org repository in section #s71 :
http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/text-align-vrl-002.htm
http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/text-align-vrl-004.htm
...
http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/text-align-vrl-016.htm
http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/text-align-vrl-018.htm
and
http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/text-align-vlr-003.htm
http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/text-align-vlr-005.htm
...
http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/text-align-vlr-017.htm
http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/text-align-vlr-0019.htm
Whiteboard: Updated links to tests are in comment 14
Reporter | ||
Comment 15•9 years ago
|
||
err... last link corrected:
http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/text-align-vlr-019.htm
You need to log in
before you can comment on or make changes to this bug.
Description
•