float and shrink-to-fit in vertical writing-modes

RESOLVED FIXED in Firefox 40

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: bugzilla, Assigned: jfkthame)

Tracking

(Blocks 1 bug, {testcase})

Trunk
mozilla40
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(6 attachments)

Reporter

Description

4 years ago
Tests
-----

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/float-shrink-to-fit-basic.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/float-shrink-to-fit-both-borders.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/float-shrink-to-fit-both-horizontal-border-padding.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/float-shrink-to-fit-016.xht

Expected results
----------------

There is no red

Explanation
-----------
In case a floated non-replaced element (or an inline-block non-replaced element) has 'height: auto' (which is the logical width) in a vertical writing-mode, then shrink-to-fit algorithm must apply to determine its used height (logical width).
CSS2.1, §10.3.5 Floating, non-replaced elements
http://www.w3.org/TR/2011/REC-CSS2-20110607/visudet.html#float-width
and CSS3 Writing-Modes, §7.1 Principles of Layout in Vertical Writing Modes
"
Layout calculation rules (such as those in CSS2.1, Section 10.3) that apply to the horizontal dimension in horizontal writing modes instead apply to the vertical dimension in vertical writing modes.
"
There are 2 other situations (CSS2.1, §10.3.7) where shrink-to-fit algorithm must also apply.
In the 4 tests, the used height (logical width) of each and all floating elements should be 0; therefore, no red should be rendered.


Notes
-----
- I have not yet submitted those tests to the test.csswg.org repository but I will, along with correspondent reference files
- Those tests' filenames will most likely change when I submit them to test.csswg.org repository
- IE11, Chrome 40.0.2214.93 and Prince 9.0.5 pass these 4 tests.
- I am using Firefox 38.0a1 buildID=20150204150323
- I use Linux 3.16.0-30-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

4 years ago
Blocks: writing-mode
Keywords: testcase
Reporter

Comment 1

4 years ago
In this test

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/float-shrink-to-fit-789.xht

(which is passed by Chrome 40.0.2214.111, IE11 and Prince 9.0.5)

the glyphs "L" and "R" appear to be not rendered at all: after dynamically setting the font-family to serif, the "L" and "R" should become viewable... but they will not. 
The logical width of the content is zero. So there is something wrong with the handling of content. If content is 123456, then only 12345 is rendered; the last glyph is not rendered. 

I can file this issue into a separate bug report...
Reporter

Comment 2

4 years ago
Better test:

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/float-shrink-to-fit-790.xht

The content (the "6" glyph and a part of "5" glyph; it depends on the installed serif font) at line-end is overlapping the blue border-bottom in Firefox 38.0. Not so in Chrome 40.0.2214.111, IE11 and Prince 9.0.5.
For a start, we still have some holdouts in nsHTMLReflowState that are using height and width instead of abstract sizes when calculating size constraints.
Assignee: nobody → smontagu
Blocks: 1077521
No longer blocks: enable-writing-mode-dev
Reporter

Comment 4

4 years ago
> - I have not yet submitted those tests to the test.csswg.org repository but
> I will, along with correspondent reference files

Done.

http://lists.w3.org/Archives/Public/public-css-testsuite/2015Mar/0001.html

> - Those tests' filenames will most likely change when I submit them to
> test.csswg.org repository

The tests at/in gtalbot.org/BrowserBugsSection/CSS3WritingModes/ use vendor-prefixed property names.

The tests' new filenames at test.csswg.org use unprefixed property names:

writing-mode: vertical-lr
 - - - - - - - - - - - - 

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vlr-003.htm

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vlr-005.htm

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vlr-007.htm

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vlr-009.htm

writing-mode: vertical-rl
 - - - - - - - - - - - - 

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vrl-002.htm

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vrl-004.htm

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vrl-006.htm

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vrl-008.htm

Both vertical writing-modes
 - - - - - - - - - - - - - 

http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-shrink-to-fit-vrl-vlr-016.htm
Depends on: 1147834
Bug 1145218 is probably also a blocker for this.
Depends on: 1145218
Assignee

Comment 6

4 years ago
It turns out the primary issue here is that nsFrame::IntrinsicISizeOffsets, which is used to accumulate border contributions to intrinsic size, hasn't been made vertical-aware. Fixing this makes all the testcases here render as expected, AFAICT.
Attachment #8587931 - Flags: review?(smontagu)
Assignee

Updated

4 years ago
Assignee: smontagu → jfkthame
Status: NEW → ASSIGNED
Assignee

Comment 7

4 years ago
Here's a testcase that demonstrates the incorrect intrinsic sizing we get (as shown by the size of the floated divs) with current code, and displays correctly once this is fixed.

(Should be possible to turn this into a reftest...)
Assignee

Comment 9

4 years ago
Here's a reftest to go with the patch above.
Attachment #8587979 - Flags: review?(smontagu)
Assignee

Comment 10

4 years ago
It turns out there's a similar problem in nsContainerFrame, although the original testcases here didn't show it. Here's a modified testcase that renders incorrectly even after the nsFrame::IntrinsicISizeOffsets patch.
Assignee

Comment 11

4 years ago
And here's the patch that fixes that second example.
Attachment #8587983 - Flags: review?(smontagu)
Assignee

Comment 14

4 years ago
(In reply to Jonathan Kew (:jfkthame) from comment #13)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=140c47a6dc0c

Note that none of the oranges in this try run are from this bug; they're the tests from bug 1150614, which was also in my queue at the time.
Comment on attachment 8587931 [details] [diff] [review]
Account for writing-mode when incorporating border widths into intrinsic size

Review of attachment 8587931 [details] [diff] [review]:
-----------------------------------------------------------------

Nice
Attachment #8587931 - Flags: review?(smontagu) → review+
Attachment #8587979 - Flags: review?(smontagu) → review+
Attachment #8587983 - Flags: review?(smontagu) → review+
Attachment #8587987 - Flags: review?(smontagu) → review+
No longer depends on: 1147834
Reporter

Comment 18

4 years ago
(In reply to Jonathan Kew (:jfkthame) from comment #10)
> Created attachment 8587980 [details]
> testcase for nsContainerFrame intrinsic size involving borders
> 
> It turns out there's a similar problem in nsContainerFrame, although the
> original testcases here didn't show it. Here's a modified testcase that
> renders incorrectly even after the nsFrame::IntrinsicISizeOffsets patch.

Jonathan,

Are you sure about that test? I ask because that would cause a noticeable difference between how inline boxes with border and padding would be rendered in vertical writing-modes versus how they are rendered in horizontal writing-mode.

CSS 1, Section 4.4 The height of lines says 
"any padding, border or margin above and below non-replaced inline elements does not influence the height of the line. In other words: if the 'line-height' is too small for the chosen padding or border, it will overlap with text on other lines."
http://www.w3.org/TR/CSS1/#the-height-of-lines

CSS 2.1, Section 10.8.1 Leading and half-leading says 
"The height of the inline box encloses all glyphs and their half-leading on each side and is thus exactly 'line-height'. Boxes of child elements do not influence this height.

Although margins, borders, and padding of non-replaced elements do not enter into the line box calculation, they are still rendered around inline boxes. This means that if the height specified by 'line-height' is less than the content height of contained boxes, backgrounds and colors of padding and borders may 'bleed' into adjoining line boxes."
http://www.w3.org/TR/CSS21/visudet.html#line-height

Tests
-----

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/sizing-inline-content-with-border-vlr-003-draft.xht

I think IE11 renders this test as I think it should be. 
Firefox 42 does not. 
Chrome 45 is very wrong in my opinion.

Here's a test I created several years ago, testing the quoted and specified statements of the spec:

http://www.gtalbot.org/BrowserBugsSection/MSIE7Bugs/NestedInlineElements.html

Gérard
Reporter

Comment 19

4 years ago
Exact same test as sizing-inline-content-with-border-vlr-003-draft but without 'float: left' :

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/sizing-inline-content-with-border-vlr-003-draft2.xht

and again, in my opinion, IE11 renders it correctly. Firefox 42 buildID=20150718201542 and Chrome 45.0.2454.12 do not.
Reporter

Comment 20

4 years ago
An interactive demo test

http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/sizing-inline-content-with-border-dhtml.xht

only working in Firefox 41+ and in Chrome 43+
Reporter

Updated

4 years ago
Flags: needinfo?(jfkthame)
Assignee

Comment 21

4 years ago
Note that these sizing-inline tests may be at least partly dependent on baselines and how vertical-align is handled. The behavior is noticeably different if you set 'text-orientation: sideways-right" on the examples, which makes baseline alignment behave more like it does in horizontal mode.

This may be related to the issues in bug 1179952; and if not, it'd be best to file a separate bug with the relevant examples - thanks.
Flags: needinfo?(jfkthame)

Comment 22

4 years ago
No, it's not related to bug 1179952 -- that's about inline-blocks, and these are just regular inlines. It's clearly an alignment bug, but also clearly a separate bug from this one.
Reporter

Comment 23

4 years ago
(In reply to Jonathan Kew (:jfkthame) from comment #21)
> Note that these sizing-inline tests may be at least partly dependent on
> baselines and how vertical-align is handled.

I do not think so. Whether dominant baseline is central or alphabetical should not affect how border and padding of inline non-replaced elements is rendered (or is supposed to be rendered) in a line box. Which baseline-alignment method is involved is one distinct and separate thing; padding and border of inline non-replaced elements should not affect (increase, stretch) line box height.

> The behavior is noticeably
> different if you set 'text-orientation: sideways-right" on the examples,
> which makes baseline alignment behave more like it does in horizontal mode.
> 
> This may be related to the issues in bug 1179952; and if not, it'd be best
> to file a separate bug with the relevant examples - thanks.

I will file a separate bug with examples on this: it's one item in my very long to-do-list now.
Reporter

Comment 24

4 years ago
> I will file a separate bug with examples on this

Bug 1220352 has been filed: "Line box height should not grow to accomodate border and padding of non-replaced inline boxes"
You need to log in before you can comment on or make changes to this bug.