1561794 - The select does not show the second digit

Status: VERIFIED FIXED

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

Description

[Karl Dubost💡 :karlcow]

Attachment: gecko, blink, WebKit (left to right)

This was initially reported on https://webcompat.com/issues/33636

https://codepen.io/webcompat/pen/RzLEva

<div>
  <select class="one">
    <option value="15" selected="">15</option>
  </select>
</div>

<div>
  <select class="bis">
    <option value="15" selected="">15</option>
  </select>
</div>

with

div { width: 50px;}
select {
	width: 100%;
	text-overflow: "";
}

.one {	padding-right: 0px;}
.bis {	padding-right: 3px;}

At the beginning this was something similar to Bug 1501908, but this has been fixed.
Whichever padding right I put for Safari and Blink, the widget size doesn't change. They seem to ignore it.

Comment 1

[Mike Taylor [:miketaylr]]

Sean, can we get someone to look into this? Etsy is a pretty popular site.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

This wfm on Linux and on the reduced test-case Nightly and Chrome chop the text, what am I missing?

Comment 3

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment: testcase

Comment 4

[Mike Taylor [:miketaylr]]

Attachment: macOS Firefox (left) vs Chrome (right)

At least on macOS, there's a pretty obvious difference.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Attachment: Chrome dev / Epi / Nightly on linux

I guess here's our answer.

Chrome's fix-up seems mac-specific. That's lame.

Comment 6

[Sean Voisen (:svoisen)]

Emilio: Do we need to emulate the lameness? Ignoring padding doesn't seem great.

Comment 7

[Emilio Cobos Álvarez [:emilio]]

I would prefer if we didn't have to, but your call. Looks like Chrome does a lot more than ignoring margin and padding, so we should probably either implement all of it or none.

The Blink behavior makes no sense. I bet this shows no padding on mac: data:text/html,<div style="-webkit-appearance: menulist; padding: 10px; border: 10px solid green">Hey</div>. :(

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1561794 - Do not crop display text of themed comboboxes due to padding. r=dbaron

This is a potential fix that I thought it was worth doing rather than
implementing Blink's platform-dependent silliness. This ensures that the display
frame always has enough space to display itself.

Note that it may still get clipped, if there's no room for both the display
frame and the button.

Needs a test if we go for this.

Comment 9

[Emilio Cobos Álvarez [:emilio]]

I think something like ^ would be better, if David agrees with me :)

Comment 10

[Emilio Cobos Álvarez [:emilio]]

Maybe Simon has opinions too.

Comment 11

[Emilio Cobos Álvarez [:emilio]]

Can somebody on a Mac confirm that this build fixes the site?

Comment 12

[Dennis Schubert [:denschub]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #11)

Can somebody on a Mac confirm that this build fixes the site?

I can confirm that your build fixes both the testcase and the original site in the webcompat report (etsy) for me on macOS 10.14.5.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/05c5e4d30587
Do not to crop display text of themed comboboxes due to padding. r=dbaron

Comment 14

[Natalia Csoregi [:nataliaCs]]

Backed out for multiple failures e.g. test_bug320799.html

backout: https://hg.mozilla.org/integration/autoland/rev/db6e0e3c4d49afb2323c83f80ab1504e4cdf7491

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunning%2Cpending%2Crunnable&revision=05c5e4d30587aeab3f566e07edda1a3e6d22c41e&selectedJob=256833508

failure log 1: dom/base/test/test_bug320799.html | Scroll width should not include dropdown contents - got 101, expected 100
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256833508&repo=autoland&lineNumber=3255

[task 2019-07-17T01:12:21.067Z] 01:12:21 INFO - 487 INFO TEST-START | dom/base/test/test_bug320799.html
[task 2019-07-17T01:12:21.068Z] 01:12:21 INFO - 488 INFO TEST-UNEXPECTED-FAIL | dom/base/test/test_bug320799.html | Scroll width should not include dropdown contents - got 101, expected 100
[task 2019-07-17T01:12:21.068Z] 01:12:21 INFO - SimpleTest.is@SimpleTest/SimpleTest.js:320:16
[task 2019-07-17T01:12:21.068Z] 01:12:21 INFO - @dom/base/test/test_bug320799.html:66:1
[task 2019-07-17T01:12:21.068Z] 01:12:21 INFO - 489 INFO TEST-PASS | dom/base/test/test_bug320799.html | Client width should not depend on the dropdown's vertical scrollbar
[task 2019-07-17T01:12:21.068Z] 01:12:21 INFO - 490 INFO TEST-UNEXPECTED-FAIL | dom/base/test/test_bug320799.html | Scroll width should not include dropdown contents - got 101, expected 100
[task 2019-07-17T01:12:21.068Z] 01:12:21 INFO - SimpleTest.is@SimpleTest/SimpleTest.js:320:16
[task 2019-07-17T01:12:21.068Z] 01:12:21 INFO - @dom/base/test/test_bug320799.html:70:1
[task 2019-07-17T01:12:21.068Z] 01:12:21 INFO - 491 INFO TEST-OK | dom/base/test/test_bug320799.html | took 59ms

failure log2: layout/reftests/forms/select/themed-select-padding-no-clip.html

00:40:55 INFO - REFTEST TEST-LOAD | file:///Z:/task_1563323570/build/tests/reftest/tests/layout/reftests/forms/select/themed-select-padding-no-clip-ref.html | 16 / 17 (94%)
00:40:55 INFO - ++DOMWINDOW == 29 (000001B5466EF800) [pid = 10284] [serial = 54] [outer = 000001B54210D980]
00:40:55 INFO - --DOCSHELL 000001FEDEA81800 == 5 [pid = 7192] [id = {63b0577e-40bb-4e2d-b5fc-d5be420cace5}] [url = chrome://browser/content/browser.xhtml]
00:40:55 INFO - --DOCSHELL 000001FEDF960800 == 4 [pid = 7192] [id = {e9aab059-5f01-48f3-a8be-7b6210bebc22}] [url = about:blank]
00:40:55 INFO - REFTEST TEST-UNEXPECTED-FAIL | file:///Z:/task_1563323570/build/tests/reftest/tests/layout/reftests/forms/select/themed-select-padding-no-clip.html == file:///Z:/task_1563323570/build/tests/reftest/tests/layout/reftests/forms/select/themed-select-padding-no-clip-ref.html | image comparison, max difference: 255, number of differing pixels: 7812

Comment 15

[Emilio Cobos Álvarez [:emilio]]

Sent an updated patch.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fb519410e98e
Do not crop display text of themed comboboxes due to padding. r=dbaron

Comment 17

[Narcis Beleuzu [:NarcisB]]

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

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider uplifting to Beta for Fx69? Please nominate it for Beta approval if so.

Comment 19

[Emilio Cobos Álvarez [:emilio]]

Yes, I think it may be worth uplifting to beta, but probably with a bit more bake time... WDYT of uplifting this in about a week or so if there are no regressions?

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Sounds fine to me!

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Has this baked sufficiently now?

Comment 22

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 9077776 [details]
Bug 1561794 - Do not crop display text of themed comboboxes due to padding. r=dbaron

Beta/Release Uplift Approval Request

• User impact if declined: Some comboboxes may seem cropped by padding.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: see comment 0
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): Simple fix to avoid cropping themed comboboxes to align a bit better with other browsers on OSX. However form control changes are always a bit risky, thus not marking as "Low".
• String changes made/needed: none

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9077776 [details]
Bug 1561794 - Do not crop display text of themed comboboxes due to padding. r=dbaron

Fix for text getting cropped in comboboxes when it shouldn't be sometimes. Baked on Nightly with no known issues so far. Approved for 69.0b10.

Comment 24

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/d6feaa7bd4ae

Comment 25

[Sorina Florean [:sflorean]]

On LG G7 Fit (Android 8.1) on both Nightly 70.0a1 (02/08) and Beta 69.0b10 the issue is still reproducible following the steps from GitHub issue (etsy.com) but on the same device, the test case is fixed. Note here that I was able to reproduce the issue (Github and testcase) on Firefox Release 68.
Emilio, should I file a new bug for the GitHub issue?

Comment 26

[Emilio Cobos Álvarez [:emilio]]

Ouch, the bug was reported on Mac, there's no mention of Android in comment 0 or following :(

The fix for Android needs to be a bit difference because on Android select works differently.

Please file another issue and ni? me, yeah
.

Comment 27

[Timea Zsoldos [:tzsoldos], Desktop QA]

I have reproduced this issue using Firefox 70.0a1 (2019.06.26) on Win 10 x64.
I can confirm this issue is fixed, I verified using Firefox latest nightly 70.0a1 and 69.0b10 using Win 10 x64, Ubuntu 18.04 x64 and macOS 10.14.

Comment 28

[Simon Pieters [:zcorpan]]

I don't have a strong opinion on this particular issue, except it seems not useful to clip the text, and I'd like consistency between browsers and OSes. ^_^

If the HTML spec should say something about this, please file an issue.