Closed
Bug 1384602
Opened 7 years ago
Closed 7 years ago
stylo: Squashed boxes in YouTrack Agile Boards view
Categories
(Core :: CSS Parsing and Computation, defect, P3)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: inejge, Assigned: bzbarsky)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
(Keywords: nightly-community)
Attachments
(8 files, 2 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
emilio
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
heycam
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20170726100241
Steps to reproduce:
Enabled Stylo (layout.css.servo.enabled = true) on FF 56.0a1 (2017-07-26) (64-bit) and went to YouTrack Agile Boards view (https://...myjetbrains.com/youtrack/agiles/...) -- can't share the URL, sorry.
Actual results:
Issue boxes are squashed horizontally, as shown in the attached screen grab.
Expected results:
The content of the boxes should be visible.
Updated•7 years ago
|
Summary: Squashed boxes in YouTrack Agile Boards view with Stylo → stylo: Squashed boxes in YouTrack Agile Boards view
Updated•7 years ago
|
Component: Untriaged → CSS Parsing and Computation
Product: Firefox → Core
Disabling Stylo and reloading the YouTrack page displays the boxes correctly. Re-enabling and reloading results in the same incorrect display.
Updated•7 years ago
|
Comment 2•7 years ago
|
||
I don't have access to the site, so reproducing is likely hard, but that's the kind of thing I suspect could be caused by bug 1384065.
Updated•7 years ago
|
Priority: -- → P2
Activating the Inspector while the boxes are incorrectly displayed and hovering over the main display area will usually redisplay the page correctly. Also, collapsing the bottom row of cards will display the rest correctly. When one of other rows is collapsed, the boxes appear, but are narrower than they should be.
Some kind of JS interaction?
Comment 4•7 years ago
|
||
(In reply to inejge from comment #3)
> Activating the Inspector while the boxes are incorrectly displayed and
> hovering over the main display area will usually redisplay the page
> correctly. Also, collapsing the bottom row of cards will display the rest
> correctly. When one of other rows is collapsed, the boxes appear, but are
> narrower than they should be.
>
> Some kind of JS interaction?
Seems like we're missing some restyle. It's not necessary but possible that it's due to JS interactions. In any case, it's impossible to diagnose without some kind of testcase...
Comment 5•7 years ago
|
||
It seems not an easily reproducible issue to me.
I just tested on my mint Linux with same Nightly build but failed to reproduce it.
I'm wondering whether we have variant configs in use.
Could you reproduce again with latest build or be able to attach about:support for reference?
Flags: needinfo?(inejge)
Reproduced with latest nightly on a demo YT project. I've attached the screenshots. Since there are fewer columns than on the real YT instance where I first noticed the glitch, box squashing is less pronounced but still easily visible. I'll attach about:support separately.
Reporter | ||
Comment 10•7 years ago
|
||
I've enabled guest access on YT, if it's OK to send the link privately please contact me by email.
Comment 11•7 years ago
|
||
So I requested the link (thanks!).
It seems that we miss a restyle / reflow of something inside a colgroup.
The layout of the page looks like:
<table>
<colgroup>
<col>...</col>
</colgroup>
</table>
The table gets resized dinamically but we fail to resize the <col> elements.
I'm trying to reduce the page to a simple test-case... Reducing angular apps is fun :/
Comment 12•7 years ago
|
||
Flushing styles of the whole page doesn't fix it, but reframing the colgroup does... Presumably we're missing an anon-box restyle somewhere?
Comment 13•7 years ago
|
||
Another option is of course that we just missed a reframe change hint or something... I'll try to figure out what the change they're making to the table is exactly.
Comment 14•7 years ago
|
||
(maybe we're supposed to post the change hints to the table wrapper? If so, we don't do that right now...)
Comment 15•7 years ago
|
||
Btw, this is pretty much confirmed.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 16•7 years ago
|
||
If there's any way to attach a testcase here (doesn't have to be minimal, just has to show the problem), that would be pretty useful...
Updated•7 years ago
|
Assignee: nobody → emilio+bugs
Updated•7 years ago
|
Priority: P2 → --
Comment 17•7 years ago
|
||
Based on Emilio's investigation in comment 12 ~ comment 15, this is pretty much the same issue in Bug 1385831. Set dependency and leave the bug open for now, so we won't forget to get back here and verify if this is a dup for Bug 1385831.
Depends on: 1385831
Comment 18•7 years ago
|
||
P3 since we think this is likely the same as bug 1385831, so that we only track the high priority in one place. We should re-test once a fix for that lands.
Priority: -- → P3
Assignee | ||
Comment 19•7 years ago
|
||
> Based on Emilio's investigation in comment 12 ~ comment 15, this is pretty much the same issue in Bug 1385831.
That's entirely unclear to me, given Emilio's comments on IRC. Investigating this now.
Assignee | ||
Comment 20•7 years ago
|
||
This testcase shows the problem both with and without stylo. I suspect that what happens with stylo is that we convert a lazy frame construction that would take the append path for Gecko into "create a frame" style changes that end up taking the insert path for all but the first one for the row (isAppend false in ContentRangeInserted, so calling InsertFrames instead of AppendFrames).
The problem is that the InsertFrames codepath for "insertion" at the end into a row in a border-collapse table is buggy. Specifically, nsCellMap::AppendCell, which looks for the first dead celldata and if found mutates it in-place to point to the new cell instead of adding more celldata. But nsCellMap::InsertCell just always inserts a new celldata. In our testcase, after the first cell has been appended to the second row it ends up with three dead cells following it (to store border-collapse border state). When we do another InsertCell(), we just insert a fifth one, and then we think the table has 5 columns and things get all confused.
Assignee | ||
Comment 21•7 years ago
|
||
Assignee | ||
Comment 22•7 years ago
|
||
This makes stylo match gecko on my minimal testcases, but NOT on the original site (though it's closer than it was)
Assignee | ||
Updated•7 years ago
|
Assignee: emilio+bugs → bzbarsky
Status: NEW → ASSIGNED
Assignee | ||
Comment 23•7 years ago
|
||
OK, I filed bug 1388898 on the underlying table layout issue.
I'll post patches that fix the minimalish testcases above (for both stylo and gecko) and the original site, by working around the cellmap breakage for now. The cellmap bits will need more thought.
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Comment 24•7 years ago
|
||
MozReview-Commit-ID: 7MyO1ZyS9zu
Attachment #8895570 -
Flags: review?(emilio+bugs)
Assignee | ||
Updated•7 years ago
|
Attachment #8895551 -
Attachment is obsolete: true
Assignee | ||
Comment 25•7 years ago
|
||
MozReview-Commit-ID: 5iOaG5UNAwG
Attachment #8895571 -
Flags: review?(cam)
Assignee | ||
Updated•7 years ago
|
Attachment #8895517 -
Attachment is obsolete: true
Updated•7 years ago
|
Attachment #8895570 -
Flags: review?(emilio+bugs) → review+
Updated•7 years ago
|
Attachment #8895571 -
Flags: review?(cam) → review+
Assignee | ||
Comment 26•7 years ago
|
||
That second changeset causes an assertion failure on layout/tables/crashtests/460637-2.xhtml because our damage area has height 9 (corresponding the the rowspan), but it's a dead rowspan, so the actual number of rows is 1. This fails the VerifyDamageRect() assertion in AddBCDamageArea called from nsTableFrame::AppendCell. I filed bug 1389295 on this.
Depends on: 1389295
Comment 27•7 years ago
|
||
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dc3333e3ff2b
part 1. When coalescing lazy frame construction reframes for stylo, do it even across comments. r=emilio
https://hg.mozilla.org/integration/autoland/rev/924d16d996e2
part 2. Convert inserts to appends on table rows if possible, because the insert codepath is rather buggy. r=heycam
Comment 28•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/dc3333e3ff2b
https://hg.mozilla.org/mozilla-central/rev/924d16d996e2
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•