Checkbox styled with margin:0 is replaced by ellipsis when next to the edge in a text-overflow block

VERIFIED FIXED in Firefox 7

Status

()

Core
Layout
--
minor
VERIFIED FIXED
6 years ago
3 years ago

People

(Reporter: Yann Brelière, Assigned: dbaron)

Tracking

({regression, testcase})

7 Branch
mozilla8
regression, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox7+ fixed)

Details

(Whiteboard: [inbound] text-overflow regression in 7, URL)

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
User Agent: Mozilla/5.0 (X11; Linux i686; rv:7.0a1) Gecko/20110704 Firefox/7.0a1
Build ID: 20110704210442

Steps to reproduce:

I have put both a checkbox input and some text in a parent having `text-overflow: ellipsis;`.


Actual results:

Whether or not the text is too long (in which case it will have a proper ellipsis at the end), the checkbox will always be replaced by an ellipsis!

You can see the html/css and the result in this test case:
http://jsfiddle.net/yanndinendal/qKAQc/


Expected results:

In Chrome, the checkbox is not replaced by an ellipsis. Only the text has an ellipsis when it is too long.
(Reporter)

Updated

6 years ago
Yeah, this is broken.  roc?

We need to track this for 7; this is a serious regression, imo.
Status: UNCONFIRMED → NEW
status-firefox7: --- → affected
tracking-firefox7: --- → ?
Component: Style System (CSS) → Layout
Ever confirmed: true
Keywords: regression
QA Contact: style-system → layout
Created attachment 543930 [details]
Testcase #1

The problem is caused by the following style rule from
http://fiddle.jshell.net/css/normalize.css

input {
	margin:0;
}
Assignee: nobody → matspal
Hmm.  Is that making the checkbox overflow to the left or something?
Yes, it appears our implementation of <input type="checkbox"> is broken in
the sense that it causes overflow when styled with margin:0.
The text-overflow implementation does the right thing in this case
and removes the [atomic inline-level] element.
> It causes overflow when styled with margin:0

Why, exactly?  What's overflowing?

If you set a negative margin, does the checkbox disappear in other UAs too?
Created attachment 543971 [details] [diff] [review]
hack

The overflow comes from nsNativeThemeGTK::GetWidgetOverflow.
The bug does not occur when I nuke it with this patch.

Hmm, I suppose we really do want theme shadows and such to actually overflow...

So, I guess we could make text-overflow take such overflow into account
(and ignore it).  Are there other (simpler) solutions or workarounds?
Created attachment 543987 [details]
Testcase #2
Attachment #543930 - Attachment is obsolete: true

Updated

6 years ago
Attachment #543987 - Attachment is patch: false
Attachment #543987 - Attachment mime type: text/plain → text/html

Updated

6 years ago
Severity: normal → minor
Keywords: testcase
OS: Other → All
Summary: Checkbox is replaced by ellipsis → Checkbox styled with margin:0 is replaced by ellipsis when next to the edge in a text-overflow block
Hmm.  Are those layout overflow or painting overflow?  I'd think that text/overflow should only consider the former, not the latter....
(Assignee)

Comment 9

6 years ago
Currently reported widget overflow affects both scrollable and visual overflow (see nsIFrame::FinishAndStoreOverflow in layout/generic/nsFrame.cpp).  I agree that it does make sense to change that -- I suppose if there are widgets for which such a change doesn't make sense, they're not reporting their size correctly.

(I agree that considering the interaction of text-overflow with overflow, and that overflow operates on scrollable overflow, I think text-overflow should operate on scrollable overflow.)
(Assignee)

Comment 10

6 years ago
Created attachment 544005 [details] [diff] [review]
patch to apply native theme overflow only to visual overflow and not scrollable

I suspect this will fix it, but I don't see the issue with my GTK theme.  Could somebody who does test this?
Created attachment 544008 [details] [diff] [review]
fix + test

Here's a somewhat less risky fix if we need it...
It ignores overflow if it's within the overflow created by the widget theme.
(In reply to comment #10)
> Could somebody who does test this?

Yes, it fixes the problem.
(Assignee)

Updated

6 years ago
Attachment #544005 - Flags: review?(bzbarsky)
dbaron's patch fixes this for me on Mac too.
Comment on attachment 544005 [details] [diff] [review]
patch to apply native theme overflow only to visual overflow and not scrollable

r=me
Attachment #544005 - Flags: review?(bzbarsky) → review+
Fwiw, I tested dbaron's patch with various themes on Linux and I couldn't
find any errors anywhere in the UI.  I pushed it together with my reftest
to Try:  http://tbpl.mozilla.org/?tree=Try&rev=1ec682161801

Updated

6 years ago
Whiteboard: text-overflow regression in 7
http://hg.mozilla.org/integration/mozilla-inbound/rev/c4d00179ed09
http://hg.mozilla.org/integration/mozilla-inbound/rev/be0a64a9427f
Assignee: matspal → dbaron
Flags: in-testsuite+
Whiteboard: text-overflow regression in 7 → [inbound] text-overflow regression in 7
(Assignee)

Comment 17

6 years ago
Thanks for landing that.
(Assignee)

Comment 18

6 years ago
Comment on attachment 544005 [details] [diff] [review]
patch to apply native theme overflow only to visual overflow and not scrollable

Requesting approval for mozilla-aurora since this has become a bit of a problem for text-overflow.

This changes the way we handle native-themed widgets (generally form controls) that report overflow beyond their bounds.  We used to allow scrolling to such overflow; but this changes things so such overflow is not considered scrollable overflow (but is still considered visual overflow so it will repaint correctly).

It's not without risk, since it does intentionally change Web-related behavior, but in general it should only change the scrollable extents by a small number of pixels, and I think it's the right thing to do and I'm comfortable making this change early in the aurora cycle.
Attachment #544005 - Attachment description: possible patch → patch to apply native theme overflow only to visual overflow and not scrollable
Attachment #544005 - Flags: approval-mozilla-aurora?

Updated

6 years ago
Attachment #544005 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

6 years ago
tracking-firefox7: ? → +
http://hg.mozilla.org/mozilla-central/rev/c4d00179ed09
http://hg.mozilla.org/mozilla-central/rev/be0a64a9427f
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7

Updated

6 years ago
Target Milestone: mozilla7 → mozilla8
(Assignee)

Comment 20

6 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/5fc8239b4d74
https://hg.mozilla.org/releases/mozilla-aurora/rev/673ab9bab7b5
status-firefox7: affected → fixed
Mozilla/5.0 (X11; Linux x86_64; rv:7.0) Gecko/20100101 Firefox/7.0

Verified this issue on F7beta1 and latest Nightly using test case 2 and an ellipsis still appears next to number 4 (all other check-boxes are displayed correctly). On Chrome, all check-boxes appear correctly. 

Checked on Windows XP, Windows 7, Ubuntu 11.04 and Mac OS X 10.6.

Should this issue be re-opened?
(Assignee)

Comment 22

6 years ago
The test next to number 4 has a negative margin, so I'd expect it to have overflow.
Setting status to Verified Fixed based on comment 22.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.