float with writing-mode orthogonal to its containing block (parent) is misplaced because it's assumed not to fit next to content preceding its placeholder in the line
Categories
(Core :: Layout: Floats, defect, P3)
Tracking
()
People
(Reporter: bugzilla, Assigned: TYLin)
References
(Blocks 2 open bugs)
Details
(Keywords: testcase)
Attachments
(7 files)
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
Bug 1323517 Part 5 - Improve the available size computation for reflowing a float. r?jfkthame,emilio
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review |
Tests with references - - - - - - - - - - - - - - http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-lft-orthog-htb-in-vlr-002.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-lft-orthog-htb-in-vlr-002-ref.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-lft-orthog-htb-in-vrl-002.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-lft-orthog-htb-in-vrl-002-ref.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-lft-orthog-vlr-in-htb-002.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-lft-orthog-vlr-in-htb-002-ref.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-lft-orthog-vrl-in-htb-002.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-lft-orthog-vrl-in-htb-002-ref.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-rgt-orthog-htb-in-vlr-003.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-rgt-orthog-htb-in-vlr-003-ref.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-rgt-orthog-htb-in-vrl-003.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-rgt-orthog-htb-in-vrl-003-ref.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-rgt-orthog-vlr-in-htb-003.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-rgt-orthog-vlr-in-htb-003-ref.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-rgt-orthog-vrl-in-htb-003.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-rgt-orthog-vrl-in-htb-003-ref.xht Explanations ------------ " The outer top of an element's floating box may not be higher than the top of any line-box containing a box generated by an element earlier in the source document. (...) A floating box must be placed as high as possible. " CSS2.2, §9.5.1 Positioning the float: the 'float' property https://www.w3.org/TR/CSS22/visuren.html#float-position Notes ----- - The tests have not yet been submitted to the CSSWG test repository; the code of these tests and the filename of these tests will change - Edge, IE11, Chrome 55.0.2883.87 and Chrome 57.0.2946.0 pass these 2 tests - I am using Firefox 53.0a1 buildID=20161214030231 - I use Linux 3.13.0-105-generic x86_64, Qt: 4.8.6, KDE 4.14.13; Kubuntu (trusty) 14.04.5 LTS - I've searched for duplicates and did not find any.
|
Reporter | |
Updated•8 years ago
|
|
|
Reporter | |
Comment 1•7 years ago
|
||
> the code of these tests and the filename of these tests will change The new tests cover each 4 orthogonal contexts and each float values: 4 orthogonal combinations: vlr-in-htb, vrl-in-htb, htb-in-vrl, htb-in-vlr 2 float values: left, right =========================== 8 tests The new tests and their associated reference files are: http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-lft-orthog-htb-in-vlr-002.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-lft-orthog-htb-in-vlr-002-ref.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-lft-orthog-htb-in-vrl-002.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-lft-orthog-htb-in-vrl-002-ref.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-lft-orthog-vlr-in-htb-002.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-lft-orthog-vlr-in-htb-002-ref.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-lft-orthog-vrl-in-htb-002.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-lft-orthog-vrl-in-htb-002-ref.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-rgt-orthog-htb-in-vlr-003.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-rgt-orthog-htb-in-vlr-003-ref.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-rgt-orthog-htb-in-vrl-003.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-rgt-orthog-htb-in-vrl-003-ref.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-rgt-orthog-vlr-in-htb-003.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-rgt-orthog-vlr-in-htb-003-ref.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-rgt-orthog-vrl-in-htb-003.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s732-float-rgt-orthog-vrl-in-htb-003-ref.xht > - The tests have not yet been submitted to the CSSWG test repository; The new tests and reference files have now been submitted to the CSSWG test repository: http://hg.csswg.org/test/rev/5fcf389266c2
|
|
Reporter | |
Updated•7 years ago
|
|
||
Comment 2•7 years ago
|
||
(In reply to Gérard Talbot from comment #1) > The new tests cover each 4 orthogonal contexts and each float values: > > 4 orthogonal combinations: vlr-in-htb, vrl-in-htb, htb-in-vrl, htb-in-vlr > 2 float values: left, right > =========================== > 8 tests These 8 testcases have been imported into dir http://searchfox.org/mozilla-central/source/layout/reftests/w3c-css/received/css-writing-modes-3.
|
|
Reporter | |
Comment 3•6 years ago
•
|
||
> These 8 testcases have been imported into dir > http://searchfox.org/mozilla-central/source/layout/reftests/w3c-css/received/ > css-writing-modes-3. Corrected link: https://searchfox.org/mozilla-central/source/testing/web-platform/tests/css/css-writing-modes/ - - - - - - - - If you visit http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/chapter-7.htm#s7.3.2 and if you can not find the tests, then do not worry. We are aware of that situation: [css-writing-modes-3] 88 tests and 69 reference files now missing from test suite https://lists.w3.org/Archives/Public/public-css-testsuite/2018Jan/0000.html For now, please use the links listed in comment 1
|
|
Reporter | |
Comment 4•6 years ago
|
||
> If you visit > > http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/ > chapter-7.htm#s7.3.2 > > and if you can not find the tests, Elika (thank you!) fixed that particular problem. The tests are now in section 7.3.1: http://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/chapter-7.htm#s7.3.1
|
|
Reporter | |
Comment 5•6 years ago
|
||
DUPLICATE of bug 1444320 I have to decide which one to close now...
|
|
Reporter | |
Comment 7•6 years ago
|
||
Submitted tests and their associated reference files * * * * * * * * * * * * * * * * * * * * * * * * * * https://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-lft-orthog-htb-in-vrl-002.htm https://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/reference/float-lft-orthog-htb-in-vrl-002-ref.htm https://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-lft-orthog-vlr-in-htb-002.htm https://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/reference/float-lft-orthog-vlr-in-htb-002-ref.htm https://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-lft-orthog-vrl-in-htb-002.htm https://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/reference/float-lft-orthog-vrl-in-htb-002-ref.htm https://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-rgt-orthog-htb-in-vlr-003.htm https://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/reference/float-rgt-orthog-htb-in-vlr-003-ref.htm https://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-rgt-orthog-htb-in-vrl-003.htm https://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/reference/float-rgt-orthog-htb-in-vrl-003-ref.htm https://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-rgt-orthog-vlr-in-htb-003.htm https://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/reference/float-rgt-orthog-vlr-in-htb-003-ref.htm https://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/float-rgt-orthog-vrl-in-htb-003.htm https://test.csswg.org/suites/css-writing-modes-3_dev/nightly-unstable/html/reference/float-rgt-orthog-vrl-in-htb-003-ref.htm
My first guess at where this would be going wrong is that for some reason this test would be failing when it should be passing and allowing us to place the float immediately.
... in particular, it seems that ComputeFloatISize is returning NS_UNCONSTRAINEDSIZE.
... which isn't at all surprising given how it works.
It seems like we really could just reflow the float before deciding whether it fits, as long as we know not to reflow it again later (or as long as we know the second reflow will be efficient -- which it should be in most cases, although unfortunately not all). It might be worth restructuring the code to reflow the float earlier and then worry about placement -- and perhaps restructure the interaction between BlockReflowInput::AddFloat and BlockReflowInput::FlowAndPlaceFloat (and maybe even also nsBlockFrame::ReflowFloat, although that may not be needed here).
|
Assignee | |
Comment 11•2 years ago
|
||
In FlowAndPlaceFloat(), condense two identical assertions about float frame
parent into one.
FlowAndPlaceFloat() calls ReflowFloat() only once, either an early reflow (when
earlyFloatReflow is true) or a late reflow (when earlyFloatReflow is
false), so the reflow status is always fresh.
|
||
Updated•2 years ago
|
|
|
Assignee | |
Comment 12•2 years ago
|
||
These are small printf statements that are not build by CI, which increase
maintenance burden.
Depends on D151203
|
|
Assignee | |
Comment 13•2 years ago
|
||
Both the method's documentation and implementation already use logical
coordinate.
Depends on D151204
|
|
Assignee | |
Comment 14•2 years ago
|
||
This is a preparation for the next part.
Depends on D151205
|
|
Assignee | |
Comment 15•2 years ago
|
||
First of all, nsBlockFrame::AdjustFloatAvailableSpace() is misleading. It
doesn't adjust the argument aFloatAvailableSpace at all, nor does it use any
fields in nsBlockFrame. It simply returns the available space in the parent
block's content area. Thus, I move it into BlockReflowState, and have it return
the available size rather than a rect because a size is sufficient for reflowing
a float.
Also, nsBlockFrame::ReflowFloat() only cares about the available size, but not
the position of the available space, so it is sufficient to pass a LogicalSize
computed by the new method ComputeAvailableSizeForFloat().
In FlowAndPlaceFloat(), there is a loop searching for a wide enough band to
place the float. We don't need to adjust availSize every time mBCoord is changed
in the loop. We can just call ComputeAvailableSizeForFloat() to get a new
available size before reflowing the float in the !earlyFloatReflow branch.
This patch shouldn't change the behavior.
Depends on D151206
|
|
Assignee | |
Comment 16•2 years ago
|
||
This patch is a preparation for the next part, and doesn't change the behavior
yet.
FlowAndPlaceFloat() is used to return true and false. This patch changes its
return value true to PlaceFloatResult::Placed and false to
PlaceFloatResult::ShouldPlaceInNextContinuation.
In the next part, we'll move the logic dealing with "below the current line
floats" into FlowAndPlaceFloat(), and make it return
PlaceFloatResult::ShouldPlaceBelowCurrentLine.
Depends on D151207
|
|
Assignee | |
Comment 17•2 years ago
|
||
The old code in AddFloat() used to call nsBlockFrame::ComputeFloatISize() to
compute a float's inline-size, compare it with current line's available
inline-size, and determine whether FlowAndPlaceFloat() should be called.
However, it doesn't handle an orthogonal float with an auto block-size.
Luckily, FlowAndPlaceFloat() already has logic dealing with orthogonal
floats (bug 1141867), so this patch defers the decision to place a float below
the current line until the float's margin inline-size is computed in
FlowAndPlaceFloat().
Depends on D151208
|
||
Comment 18•2 years ago
|
||
Pushed by tlin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/48331f7fdc7d Part 1 - Remove a duplicate assertion, and assert a fresh reflow status in ReflowFloat(). r=jfkthame https://hg.mozilla.org/integration/autoland/rev/6953d70bc28e Part 2 - Remove float debug log that requires manual #define. r=jfkthame https://hg.mozilla.org/integration/autoland/rev/aebe719d769b Part 3 - Rename GetLowestFloatTop to LowestFloatBStart. r=jfkthame https://hg.mozilla.org/integration/autoland/rev/d6a37445d86b Part 4 - Pass a dummy rect into ReflowBlock() when reflowing a float. r=jfkthame https://hg.mozilla.org/integration/autoland/rev/e53ad6f93a0f Part 5 - Improve the available size computation for reflowing a float. r=jfkthame https://hg.mozilla.org/integration/autoland/rev/a70aa231b13e Part 6 - Change FlowAndPlaceFloat()'s return value to a tri-state enum. r=jfkthame https://hg.mozilla.org/integration/autoland/rev/d08bab0259f7 Part 7 - Fix the placement of an orthogonal float with an auto block-size. r=jfkthame
|
||
Comment 19•2 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/48331f7fdc7d
https://hg.mozilla.org/mozilla-central/rev/6953d70bc28e
https://hg.mozilla.org/mozilla-central/rev/aebe719d769b
https://hg.mozilla.org/mozilla-central/rev/d6a37445d86b
https://hg.mozilla.org/mozilla-central/rev/e53ad6f93a0f
https://hg.mozilla.org/mozilla-central/rev/a70aa231b13e
https://hg.mozilla.org/mozilla-central/rev/d08bab0259f7
|
||
Updated•2 years ago
|
Description
•