963970 - Arrow of drop-down list should not be affected by padding

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect, P4)

Description

[Sebastian Zartner [:sebo]]

Attachment: dropDownListArrow.html

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

Comment 1

[Sebastian Zartner [:sebo]]

Attachment: dropDownListArrow.png

Expected display of the drop down arrow in Windows 8.1

Comment 2

[Alex Henrie]

Attachment: Proposed patch

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.

Comment 3

[Mats Palmgren (inactive)]

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.

Comment 4

[Mats Palmgren (inactive)]

The patch breaks <select style="border:solid;"> too.
(I'm testing this on Linux, in case it matters.)

Comment 5

[Alex Henrie]

Attachment: Proposed patch (try 2)

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?

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Try adding some dumping code to nsComboboxControlFrame::Reflow to dump the position of the button, and push that to try.

Comment 7

[Alex Henrie]

Attachment: Proposed patch (try 3)

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.

Comment 8

[Alex Henrie]

Never mind, adding -moz-appearance:menulist to the reference would defeat the purpose of the test.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 10

[Alex Henrie]

Attachment: Proposed patch (try 4)

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?

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(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.

Comment 12

[Alex Henrie]

Attachment: Proposed patch (try 5)

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?

Comment 13

[Alex Henrie]

Requesting checkin of attachment 8387161 [details] [diff] [review]

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cfbcdc591ccd

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cfbcdc591ccd

Comment 16

[Sebastian Zartner [:sebo]]

Works fine for me testing on Win 8.1 and Ubuntu 12.04.

Sebastian

Comment 17

[Kohei Yoshino [:kohei]]

https://developer.mozilla.org/en-US/Firefox/Releases/30/Site_Compatibility#CSS