Closed
Bug 963970
Opened 11 years ago
Closed 11 years ago
Arrow of drop-down list should not be affected by padding
Categories
(Core :: Layout: Form Controls, defect, P4)
Core
Layout: Form Controls
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: sebo, Assigned: alexhenrie24)
References
Details
(Keywords: dev-doc-complete, site-compat, testcase)
Attachments
(3 files, 4 obsolete files)
The display of the arrow inside a drop-down list should not be affected by the padding set for the <select> element.
This was first mentioned in comment 144 of bug 157846.
Sebastian
Reporter | ||
Comment 1•11 years ago
|
||
Expected display of the drop down arrow in Windows 8.1
Assignee | ||
Comment 2•11 years ago
|
||
I agree that this is a problem. Putting the combobox button inside the padding is ugly and doesn't match the native themes or Chrome's rendering.
I've written a patch that resolves the issue everywhere but B2G: https://tbpl.mozilla.org/?tree=Try&rev=bc6beacc444d
If Robert lends a hand again, I think we can get this resolved pretty quick.
Attachment #8365646 -
Flags: feedback?(roc)
Assignee | ||
Updated•11 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 3•11 years ago
|
||
The proposed fix breaks this test:
data:text/html,<select%20style="padding-top:10%;border:solid"><option>XXXXXXXXXXXXXXXXXXXXXX</select>
Maybe the problem is the switch from Used values to Computed values?
Please add a <select style=direction:rtl> to the test.
Updated•11 years ago
|
Severity: normal → minor
Keywords: testcase
OS: Windows 8.1 → All
Priority: -- → P4
Hardware: x86_64 → All
Comment 4•11 years ago
|
||
The patch breaks <select style="border:solid;"> too.
(I'm testing this on Linux, in case it matters.)
Assignee | ||
Comment 5•11 years ago
|
||
Well, I'm a bit closer. This patch includes an RTL test and works fine with the two examples you gave. Thanks for the suggestion about computed vs. used values.
Unfortunately, this patch causes a problem with layout/reftests/bugs/557087-2.html on Windows: https://tbpl.mozilla.org/?tree=Try&rev=0418cb20316f
I've tried everything I could think of with no success. Any more suggestions?
Attachment #8365646 -
Attachment is obsolete: true
Attachment #8365646 -
Flags: feedback?(roc)
Attachment #8384874 -
Flags: feedback?(roc)
Attachment #8384874 -
Attachment is patch: true
Try adding some dumping code to nsComboboxControlFrame::Reflow to dump the position of the button, and push that to try.
Assignee | ||
Comment 7•11 years ago
|
||
After some more trial and error, I've found a combination that only fails on Android and B2G: https://tbpl.mozilla.org/?tree=Try&rev=302eec78265b
I'm thinking I should try setting -moz-appearance:menulist on the reference divs and see if that resolves the problem. But because I'm not set up to test Android or B2G, it's hard for me to tell.
Attachment #8384874 -
Attachment is obsolete: true
Attachment #8384874 -
Flags: feedback?(roc)
Attachment #8385461 -
Flags: feedback?(roc)
Assignee | ||
Comment 8•11 years ago
|
||
Never mind, adding -moz-appearance:menulist to the reference would defeat the purpose of the test.
That failure is just because mobile/android/themes/core/content.css has:
/* Override inverse OS themes */
select,
textarea,
button,
xul|button,
* > input:not([type="image"]) {
-moz-appearance: none !important; /* See bug 598421 for fixing the platform */
border-radius: 3px;
}
and your reference DIVs don't.
You can work around this by adding border-radius:0 to the selects in padding-button-placement.html.
Assignee | ||
Comment 10•11 years ago
|
||
Try 4: https://tbpl.mozilla.org/?tree=Try&rev=b3f5a71b3a58
Android is happy now (thanks for the tip!), but B2G ICS isn't. From the try server's screenshots, it looks like it gets caught in the middle of rendering.
I tried adding needs-focus to the test and it still failed on B2G ICS: https://tbpl.mozilla.org/?tree=Try&rev=e389bd413f00
I tried adding a 1-second delay to the test and it still failed on B2G ICS: https://tbpl.mozilla.org/?tree=Try&rev=c301e965550d
Fixing this bug also fixes bug 893509, because the NS_ASSERTION is removed entirely.
For the record, the fuzzy(1,4) is for Mac OS X: https://tbpl.mozilla.org/?tree=Try&rev=f54627d918ae
Robert, you've been really helpful...do you have any more insights here?
Attachment #8385461 -
Attachment is obsolete: true
Attachment #8385461 -
Flags: feedback?(roc)
Attachment #8386596 -
Flags: feedback?(roc)
(In reply to Alex Henrie from comment #10)
> Try 4: https://tbpl.mozilla.org/?tree=Try&rev=b3f5a71b3a58
>
> Android is happy now (thanks for the tip!), but B2G ICS isn't. From the try
> server's screenshots, it looks like it gets caught in the middle of
> rendering.
> I tried adding needs-focus to the test and it still failed on B2G ICS:
> https://tbpl.mozilla.org/?tree=Try&rev=e389bd413f00
Those try-pushes don't have the border-radius:0 fix.
They show two failures but the second one is bogus. I know it looks like we only rendered half the page but I think that's just logging stuff going crazy and being misinterpreted by the reftest analyzer. So focus on the first failure.
Other than the border-radius issue, the problem seems to be that the black borders aren't properly covering the drop-down button. Try making the covering abs-pos DIVs z-index:1? Here's what content.css has:
select > button {
border-width: 0px !important;
margin: 0px !important;
padding: 0px !important;
border-radius: 0;
color: #414141;
background-image: radial-gradient(at bottom left, #bbbbbb 40%, #f5f5f5), url(arrow.svg) !important;
background-color: transparent;
background-position: -15px center, 4px center !important;
background-repeat: no-repeat, no-repeat !important;
background-size: 100% 90%, auto auto;
-moz-binding: none !important;
position: relative !important;
font-size: inherit;
}
So it makes sense that the button would be higher in z-order than the abs-pos DIV in your testcase. Which is a bug, really, but not your problem. so add the z-index:1.
Assignee | ||
Comment 12•11 years ago
|
||
Try 5: https://tbpl.mozilla.org/?tree=Try&rev=847dddda4f45
Sorry, I did have border-radius:0 in https://tbpl.mozilla.org/?tree=Try&rev=f54627d918ae but then I accidentally reverted it. At any rate, it's working now, and your z-index fix worked too. Can you see any other problems?
Assignee: nobody → alexhenrie24
Attachment #8386596 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8386596 -
Flags: feedback?(roc)
Attachment #8387161 -
Flags: review?(roc)
Attachment #8387161 -
Flags: review?(roc) → review+
Assignee | ||
Comment 13•11 years ago
|
||
Requesting checkin of attachment 8387161 [details] [diff] [review]
Keywords: checkin-needed
Comment 14•11 years ago
|
||
Flags: in-testsuite+
Keywords: checkin-needed
Comment 15•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Reporter | ||
Comment 16•11 years ago
|
||
Works fine for me testing on Win 8.1 and Ubuntu 12.04.
Sebastian
Comment 17•10 years ago
|
||
Keywords: dev-doc-complete,
site-compat
You need to log in
before you can comment on or make changes to this bug.
Description
•