Closed Bug 458231 Opened 13 years ago Closed 12 years ago

Frame overflow plays havoc with size of <label> or <description>

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.2
Tracking Status
status1.9.2 --- beta4-fixed
status1.9.1 --- ?

People

(Reporter: neil, Assigned: neil)

References

()

Details

(Keywords: testcase)

Attachments

(4 files)

When a <label> or <description> contains HTML content which paints outside of its boundaries (which I believe you call overflow, not to be confused with the CSS overflow property), then they miscompute their dimensions.

A simple test case that applies a 40px outline makes the label's height 40px more than it should be. The widths I see make no sense to me at all, although strangely when opening the test case as chrome only the window's height is affected; the window's width does not change (but it still paints incorrectly.)

When I try to apply a 40px text shadow, the height is now 80px more than the expected value; the widths are at least consistently strange...
Attached file Test case
OK, so I debugged this a bit, and it turns out that <label> works by asking its area frame for its overflow area, and sizing itself based on that, because for some unknown reason its area frame's desired size is only the height of the first line of text (although I stepped into the reflow for the <div> which did know the height, which is not unreasonable since the border is drawn correctly.)

The code in question is nsFrame::BoxReflow just after the call to Reflow.
Attached patch Proposed patchSplinter Review
By allowing the label unconstrained height during reflow, we don't need to worry about fixing up the height after reflow, so that code just dies.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #341754 - Flags: superreview?(roc)
Attachment #341754 - Flags: review?(dbaron)
Much as I'd like to do this, this is a major change to the behaviour of XUL layout that I'm afraid to make without considerable thought.
Flags: blocking1.9.1?
Target Milestone: --- → mozilla1.9.1b2
(In reply to comment #4)
> Much as I'd like to do this, this is a major change to the behaviour of XUL
> layout that I'm afraid to make without considerable thought.

That says blocking- to me ...
Flags: blocking1.9.1? → blocking1.9.1-
Flags: wanted1.9.1?
Flags: wanted1.9.1? → wanted1.9.1-
Flags: wanted1.9.2?
Blocks: 479957
Attachment #341754 - Flags: superreview?(roc)
Attachment #341754 - Flags: superreview+
Attachment #341754 - Flags: review?(dbaron)
Attachment #341754 - Flags: review+
Pushed changeset 176f8cd08078 to mozilla-central.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Backed out in changeset 859496f90e66 - you failed http://mxr.mozilla.org/mozilla-central/source/layout/reftests/bugs/386470-1a.html?force=1 twice on all three platforms, putting some space above the lime div (e.g. http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1235865020.1235870023.12914.gz#err0)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to comment #9)
> Backed out in changeset 859496f90e66 - you failed
> twice on all three platforms, putting some space above the lime div
Weird. Especially as I can't seem to create a reduced test case :-(
Apparently this patch changes the computed baseline; the amount of offset depends on the font height.

Or is this something to do with overflow, i.e. I need to check for something that wants to overflow and therefore should have their size constrained?
On second thoughts it doesn't seem to be overflow; removing the overflow style doesn't seem to help.
Test 386470-1a fails without this extra change.
Attachment #382525 - Flags: superreview?(roc)
Attachment #382525 - Flags: review?(roc)
Attachment #382525 - Flags: superreview?(roc)
Attachment #382525 - Flags: superreview+
Attachment #382525 - Flags: review?(roc)
Attachment #382525 - Flags: review+
Blocks: 497829
Pushed changeset 6eb0ddec35a0 to mozilla-central.
Status: REOPENED → RESOLVED
Closed: 13 years ago12 years ago
Resolution: --- → FIXED
From what I read and see in tests, this was not applied to 1.9.1, so fixing Target Milestone field.

This causes bug 526918 in Thunderbird 3.0 based on 1.9.1. I'll attaching the backported patch.
Target Milestone: mozilla1.9.1b2 → mozilla1.9.2
Blocks: 526918
Should this land on 1.9.1 you would also want to back out the workarounds for the XP Classic native theme disabled checkboxes or radio buttons.
You need to log in before you can comment on or make changes to this bug.