Closed Bug 914919 Opened 8 years ago Closed 1 month ago
"Assertion failure: a
XMost >= x" with position:sticky and huge width
With: user_pref("layout.css.sticky.enabled", true); Assertion failure: aXMost >= x, at BaseRect.h:285 It might be best to downgrade this to a warning until bug 765861 is fixed.
I have partially-reduced testcases for most of the other BaseRect.h assertions as well.
Yeah, I don't doubt they're too hard to trigger. I had wondered at the use of MOZ_ASSERT there, so downgrading them for now sounds fine to me.
(In reply to Jesse Ruderman from comment #0) > It might be best to downgrade this to a warning until bug 765861 is fixed. I'd lean towards downgrading to (non-fatal) NS_ASSERTION for now. The assertions are there per a review comment from roc over in bug 741682 comment 37, and I know he prefers non-fatal assertions, so I'm sure he'd be fine with that change.
Attachment #803366 - Flags: review?(dholbert) → review+
Let's double-check. https://tbpl.mozilla.org/?tree=Try&rev=5cb6567a32a5
Odd to leave something as an assertion when we know how to make it fail. roc, what do you think?
Yes, this is a bug that we should fix rather than just downgrading the assertion. However, we have to evaluate the priority, and the reality is that this is probably not high priority. It doesn't seem high priority enough for a fatal assertion at least.
Cool. This is checkin-needed, then.
Increased the assertion range to 1-3 for winWidget in https://hg.mozilla.org/integration/mozilla-inbound/rev/6de097e252be since your first Win8 hit three, time will tell whether I should have done it for everything.
And because no attempt to minimize annotations goes unpunished, 1-3 for everyone in http://hg.mozilla.org/integration/mozilla-inbound/rev/557c33ea680d once a Linux32 run did three.
https://hg.mozilla.org/mozilla-central/rev/e67dad88f860 https://hg.mozilla.org/mozilla-central/rev/6de097e252be https://hg.mozilla.org/mozilla-central/rev/557c33ea680d
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Hrm, is NS_ASSERTION allowed in gfx/2d?
No, good point. But we don't have non-fatal assertions in MFBT yet. We should. We agreed to add MOZ_NONFATAL_ASSERT a while back.
I'm somewhat tempted to back this out, since it turns out including nsDebug.h before Skia headers can cause some pain, due to a "#define free" that nsDebug inadvertantly pulls in. (see bug 924768 comment 4 and 6).
Sounds fine to me.
OK, backed out per comment 17 (which builds on comment 15): https://hg.mozilla.org/integration/mozilla-inbound/rev/5f9cb0332e3c Not sure what the best way forward is here. Fortunately, this bug doesn't affect real-world behavior at all (recall that the patch was just an assertion-softening). But if this bug blocks fuzzers, maybe we should just disable these assertions, or prioritize a MOZ_NONFATAL_ASSERT implementation so that we can soften the assertions in a more kosher way.
Clearing assignee (from when this was formerly-closed), since this doesn't need to be on Corey's plate, since his internship is over. (though Corey, if you have other ideas for how to attack this, feel free to reclaim it. :) Otherwise, someone else can probably take it, depending on prioritization.)
Assignee: corey → nobody
The testcase from comment 0 still reproduces in m-c rev 20171011-f3e939a81ee1, and this comes up fairly often while fuzzing. Can this assertion be softened yet?
Status: REOPENED → RESOLVED
Closed: 8 years ago → 1 month ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.