Closed Bug 1561794 Opened 5 months ago Closed 4 months ago

The select does not show the second digit

Categories

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

68 Branch
Unspecified
Android
defect

Tracking

()

VERIFIED FIXED
mozilla70
Webcompat Priority P2
Tracking Status
firefox69 --- verified
firefox70 --- verified

People

(Reporter: karlcow, Assigned: emilio)

References

(Blocks 1 open bug, )

Details

Attachments

(5 files)

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.

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

Webcompat Priority: ? → P2
Flags: needinfo?(svoisen)

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

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

I guess here's our answer.

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

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

Flags: needinfo?(svoisen) → needinfo?(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>. :(

Flags: needinfo?(emilio)

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.

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

Maybe Simon has opinions too.

Flags: needinfo?(zcorpan)
Attachment #9077776 - Attachment description: Bug 1561794 - Do not to crop display text due to padding. r=dbaron → Bug 1561794 - Do not to crop display text of themed comboboxes due to padding. r=dbaron

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

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

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/05c5e4d30587
Do not to crop display text of themed comboboxes due to padding. r=dbaron

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

Flags: needinfo?(emilio)

Sent an updated patch.

Assignee: nobody → emilio
Flags: needinfo?(emilio)
Attachment #9077776 - Attachment description: Bug 1561794 - Do not to crop display text of themed comboboxes due to padding. r=dbaron → Bug 1561794 - Do not crop display text of themed comboboxes due to padding. r=dbaron
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fb519410e98e
Do not crop display text of themed comboboxes due to padding. r=dbaron
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70

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

Flags: needinfo?(emilio)
Flags: in-testsuite+

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?

Flags: needinfo?(emilio) → needinfo?(ryanvm)

Sounds fine to me!

Flags: needinfo?(ryanvm)

Has this baked sufficiently now?

Flags: needinfo?(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
Flags: needinfo?(emilio)
Attachment #9077776 - Flags: approval-mozilla-beta?
Flags: qe-verify+

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.

Attachment #9077776 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

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?

Flags: qe-verify+ → needinfo?(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
.

Flags: needinfo?(emilio)
Blocks: 1571399

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.

Status: RESOLVED → VERIFIED

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.

Flags: needinfo?(zcorpan)
You need to log in before you can comment on or make changes to this bug.