Closed Bug 1229999 Opened 4 years ago Closed 4 years ago
[css-grid] Assertion failure: "invalid start line" with grid, rel & abs pos
Assertion failure: aStart >= nsStyleGridLine::kMinLine && aStart <= nsStyleGridLine::kMaxLine (invalid start line), at layout/generic/nsGridContainerFrame.h:139
The problem is that ResolveAutoPosition is a bit too permissive: http://hg.mozilla.org/mozilla-central/annotate/e02b17a2b5b8/layout/generic/nsGridContainerFrame.h#l169 The kTranslatedMaxLine there is large enough that even with a maximum number of implicit tracks before line 1, it still allows auto-placed items up to kMaxLine in the untranslated grid. The problem is that with no, or just a few, implicit lines before line 1, it allows lines that would be > kMaxLine in the untranslated grid. This isn't much of a problem - we can deal with kTranslatedMaxLine lines just fine, and the layout is correct. It's just that we clamp definite lines numbers and auto-line numbers a bit differently. The assertion occurs because before we resolve lines for abs.pos. children (after auto-placement) we untranslate the grid and that leads to some lines being > kMaxLine which when we create the LineRange for the abs.pos. item in this case asserts in the ctor: http://hg.mozilla.org/mozilla-central/annotate/e02b17a2b5b8/layout/generic/nsGridContainerFrame.h#l129 This patch seems like the most correct solution since it treats auto-placed items equally with definite-placed items w.r.t clamping. There's a small runtime cost here though. An alternative solution would be to just relax this assertion, but only for abs.pos. children.
Assignee: nobody → mats
Attachment #8695897 - Flags: review?(dholbert)
Summary: Assertion failure: "invalid start line" with grid, rel & abs pos → [css-grid] Assertion failure: "invalid start line" with grid, rel & abs pos
Comment on attachment 8695897 [details] [diff] [review] fix Review of attachment 8695897 [details] [diff] [review]: ----------------------------------------------------------------- Commit message nit: > Clamp auto-placed lines the where kMaxLine is in the translated grid. r=dholbert "the where" seems like a typo. ::: layout/generic/nsGridContainerFrame.h @@ +166,5 @@ > MOZ_ASSERT(IsAuto(), "Why call me?"); > mStart = aStart; > mEnd += aStart; > // Clamping per http://dev.w3.org/csswg/css-grid/#overlarge-grids : > + uint32_t translatedMax = aExplicitGridOffset + nsStyleGridLine::kMaxLine; Can you extend the comment slightly to explain the new behavior? (Right now the comment sounds like one can find an explanation for this aExplicitGridOffset addition in the #overlarge-grids spec section; but I don't think it's really clear.) Maybe just add a parenthetical, so we end up with e.g.: // Clamping to kMaxLine (in the explicit grid), per // http://dev.w3.org/csswg/css-grid/#overlarge-grids : Also: might be worth indicating in this function's documentation that |aStart| is a line-index in the *implicit* (untranslated) grid. (It is, right?) This would help make the aExplicitGridOffset parameter & addition more obviously-correct, too. (and the old kTranslatedMaxLine comparison obviously-bogus, since it's from a different coordinate space)
Attachment #8695897 - Flags: review?(dholbert) → review+
You need to log in before you can comment on or make changes to this bug.