Closed Bug 914919 Opened 11 years ago Closed 3 years ago

"Assertion failure: aXMost >= x" with position:sticky and huge width


(Core :: Layout: Positioned, defect)

Not set



Tracking Status
firefox26 --- fixed
firefox27 --- affected


(Reporter: jruderman, Unassigned)



(Keywords: assertion, testcase)


(3 files)

Attached file testcase
  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.
Attached file stack
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.
Assignee: nobody → cford
Attachment #803366 - Flags: review?(dholbert) → review+
Odd to leave something as an assertion when we know how to make it fail. roc, what do you think?
Flags: needinfo?(roc)
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.
Flags: needinfo?(roc)
Cool. This is checkin-needed, then.
Keywords: checkin-needed
Increased the assertion range to 1-3 for winWidget in 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 once a Linux32 run did three.
Thanks, philor!
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.
Depends on: 924768
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):

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.
Resolution: FIXED → ---
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
Flags: in-testsuite+ → in-testsuite?
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?

Hey Jesse,
Can you still reproduce this issue or should we close it?

Flags: needinfo?(jruderman)

Looks like the assertion in question was removed in bug 1428348.

Closed: 11 years ago3 years ago
Flags: needinfo?(jruderman)
Resolution: --- → WORKSFORME

(and I confirmed that I don't get any assertions in a debug build with the attached testcase)

You need to log in before you can comment on or make changes to this bug.