Open Bug 1088261 Opened 10 years ago Updated 2 years ago

Make WritingModes assert when NS_UNCONSTRAINEDSIZE is passed as container width

Categories

(Core :: Layout: Text and Fonts, defect)

defect

Tracking

()

People

(Reporter: smontagu, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Attached patch Patch (deleted) — Splinter Review
To prevent and help pin down things like bug 1088151, we need to assert when passing NS_UNCONSTRAINED size as aContainerWidth to WritingModes methods
Attachment #8510555 - Flags: review?(jfkthame)
Attachment #8510555 - Flags: review?(jfkthame) → review+
the patch failed to apply: patching file layout/generic/WritingModes.h Hunk #10 FAILED at 1453 1 out of 11 hunks FAILED -- saving rejects to file layout/generic/WritingModes.h.rej patch failed, unable to continue (try -v) patch failed, rejects left in working dir errors during apply, please fix and refresh unconstrained.width.diff could you take a look, thanks!
Assignee: nobody → smontagu
Flags: needinfo?(smontagu)
Keywords: checkin-needed
Sorry about that, the version on the try push at https://hg.mozilla.org/try/rev/0c9d39ffbd48 is rebased to current inbound. But the new assertion is turning up on crashtests in the try push, so this will have to hold off for checkin anyway. Bug 1088547 will presumably catch at least some of the cases.
Depends on: 1088547
Flags: needinfo?(smontagu)
(In reply to Simon Montagu (PTO 2014-10-26 - 2014-11-10 not reading bugmail) :smontagu from comment #3) > But the new assertion is turning up on crashtests in the try push, so this > will have to hold off for checkin anyway. Bug 1088547 will presumably catch > at least some of the cases. I've started a try job that includes this together with bug 1088547; if that comes up clear, I'll go ahead and land it next time I'm ready to push.
Nope, still lots of assertions. So we'll need to investigate further before this patch lands.
It turns out that most (possibly all?) of the crashtest assertions this introduces are "expected", in the sense that they're not due to a code bug whereby we incorrectly pass NS_UNCONSTRAINEDSIZE; they're due to the use of huge dimensions in the testcases, so that we really do have a container width that exceeds NS_MAXSIZE. (Compare various other places in layout that issue warnings such as "have unconstrained inline-size; this should only result from very large sizes, not attempts at intrinsic inline-size calculation". Well, these testcases *do* have "very large sizes" that appear to be "unconstrained" as far as layout is concerned.) If we make the assertion a bit narrower, so that it tests aContainerWidth != NS_MAXSIZE rather than aContainerWidth < NS_MAXSIZE, a few of the testcases no longer assert, but several of them still do: https://tbpl.mozilla.org/?tree=Try&rev=53fa4c9db9da. So either we can't have this assertion after all, or we'll need to annotate those as "expected" assertions (after checking each testcase to make sure that's really what's causing it; I haven't examined all of them yet).

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.

Assignee: smontagu → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: