First character is clipped in units dropdown-menu in Accuweather Settings (text-transform: capitalize not affecting <select> intrinsic size)
Categories
(Core :: Layout: Form Controls, defect)
Tracking
()
People
(Reporter: dholbert, Assigned: jfkthame)
References
(Regression)
Details
(Keywords: regression)
Attachments
(6 files)
screenshot of bug on Accuweather site
STR:
- Load attached testcase.
ACTUAL RESULTS:
The "M" in "Metric" is clipped on the left side.
EXPECTED RESULTS:
No such clipping.
Firefox 130.0a1 (2024-08-04) (64-bit) gives me ACTUAL RESULTS, on Ubuntu 24.04 (Linux)
Chrome 129.0.6628.3 (Official Build) dev (64-bit) gives me EXPECTED RESULTS.
Original site here is https://www.accuweather.com/en/settings (which defaults to "Metric" in my current location, but presumably you can trigger the bug if you manually select "Metric" in another location too).
|
Reporter | |
Comment 1•3 months ago
|
||
|
|
Reporter | |
Comment 2•3 months ago
|
||
|
|
Reporter | |
Updated•3 months ago
|
|
||
Comment 3•3 months ago
|
||
First regression window:
The first vertical line of M is truncated.
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=605cc3e4f2555c8f9473b7fa7c5ffe7e0c4b6ee3&tochange=79c24b3fc675c3e6e2e62a21edd78dd224d4c9ed
Second regression window:
Truncated just like the screenshot.
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d51fbf6f8ee1cb294df35e286ef5101a19b160c6&tochange=c5bd0893afc5027660813da62f702d7265dec04a
|
|
Reporter | |
Comment 4•3 months ago
|
||
Thanks, Alice0775 White!
For what it's worth, I can reproduce on Android as well (at the original site and also using the testcase). Haven't tested other platforms, but I assume it's platform-independent.
|
||
Comment 5•3 months ago
|
||
The real bug is that nsCaseTransformTextRunFactory::TransformString doesn't deal with text-transform: capitalize here when there's no text run (like when we're measuring the intrinsic size of the <select>).
Minimal test-case:
<!doctype html>
<style>
select {
text-transform: capitalize;
appearance: none;
padding: 0;
border: 1px solid;
}
</style>
<select>
<option>x x x x x</option>
</select>
Jonathan, do you know how terrible would it be to make it work? At a glance it seems to rely on capitalization being set elsewhere, but maybe we can change it by an extra input or something.
|
Assignee | |
Comment 6•3 months ago
|
||
Hmm, IIRC currently we set the capitalization flag during text-run construction, when we scan the frame tree to collect text that belongs in a single run (which may span multiple DOM elements).
I suppose if TransformString is not given a text-run as input, we could just assume it is operating on an isolated fragment of text with no context to consider.
(As an aside, I think this is an inappropriate use of the property: in general it's wrong to force capitalization on these unit abbreviations, as seen in the screenshot. I'm surprised any designer thought that was a good presentation. But that doesn't excuse the Firefox bug, obviously!)
|
|
Assignee | |
Comment 7•3 months ago
|
||
|
||
Updated•3 months ago
|
|
|
Assignee | |
Comment 8•3 months ago
|
||
Something like this should work here; it borrows the nsLineBreaker code to decide which characters get capitalization applied, rather than reimplementing that separately. Not very elegant, but serves the purpose.
We should be able to turn comment 5 into a reftest for this, I guess.
|
|
Assignee | |
Comment 9•3 months ago
|
||
|
||
Comment 10•3 months ago
|
||
|
||
Comment 12•3 months ago
|
||
Backed out for causing non-unified build bustages on nsTextRunTransformations.cpp.
[task 2024-08-05T19:26:37.095Z] 19:26:37 INFO - gmake[4]: Entering directory '/builds/worker/workspace/obj-build/layout/generic'
[task 2024-08-05T19:26:37.098Z] 19:26:37 INFO - /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang++ -isysroot /builds/worker/fetches/MacOSX14.4.sdk -mmacosx-version-min=10.15 -stdlib=libc++ --target=x86_64-apple-darwin -o nsTextRunTransformations.o -c -I/builds/worker/workspace/obj-build/dist/stl_wrappers -fvisibility=hidden -fvisibility-inlines-hidden -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DNDEBUG=1 -DTRIMMED=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DMOZ_SUPPORT_LEAKCHECKING -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/layout/generic -I/builds/worker/workspace/obj-build/layout/generic -I/builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders -I/builds/worker/checkouts/gecko/ipc/chromium/src -I/builds/worker/checkouts/gecko/layout/base -I/builds/worker/checkouts/gecko/layout/forms -I/builds/worker/checkouts/gecko/layout/painting -I/builds/worker/checkouts/gecko/layout/style -I/builds/worker/checkouts/gecko/layout/tables -I/builds/worker/checkouts/gecko/layout/xul -I/builds/worker/checkouts/gecko/docshell/base -I/builds/worker/checkouts/gecko/dom/base -I/builds/worker/checkouts/gecko/dom/html -I/builds/worker/checkouts/gecko/dom/xul -I/builds/worker/checkouts/gecko/gfx/cairo/cairo/src -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h -fno-rtti -pthread -fno-sized-deallocation -fno-aligned-new -ffunction-sections -fdata-sections -fno-math-errno -fno-exceptions -fPIC -fcrash-diagnostics-dir=/builds/worker/artifacts -gdwarf-4 -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O3 -fno-omit-frame-pointer -funwind-tables -Werror -Wall -Wbitfield-enum-conversion -Wempty-body -Wformat-type-confusion -Wignored-qualifiers -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtautological-constant-in-range-compare -Wtype-limits -Wno-error=tautological-type-limit-compare -Wunreachable-code -Wunreachable-code-return -Wunused-but-set-parameter -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wno-range-loop-analysis -Wenum-compare-conditional -Wenum-float-conversion -Wno-deprecated-anon-enum-enum-conversion -Wno-deprecated-enum-enum-conversion -Wno-deprecated-this-capture -Wcomma -Wimplicit-fallthrough -Wduplicate-method-arg -Wduplicate-method-match -Wmissing-method-return-type -Wobjc-signed-char-bool -Wsemicolon-before-method-body -Wsuper-class-method-mismatch -Wstring-conversion -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wno-error=atomic-alignment -Wno-error=deprecated-builtins -Wformat -Wformat-security -Wno-psabi -Wthread-safety -Werror=unguarded-availability-new -Wno-error=builtin-macro-redefined -Wno-vla-cxx-extension -Wno-unknown-warning-option -fno-strict-aliasing -ffp-contract=off -MD -MP -MF .deps/nsTextRunTransformations.o.pp /builds/worker/checkouts/gecko/layout/generic/nsTextRunTransformations.cpp
[task 2024-08-05T19:26:37.099Z] 19:26:37 ERROR - /builds/worker/checkouts/gecko/layout/generic/nsTextRunTransformations.cpp:709:28: error: use of undeclared identifier 'nsLineBreaker'; did you mean 'ULineBreak'?
[task 2024-08-05T19:26:37.099Z] 19:26:37 INFO - 709 | doCapitalize = nsLineBreaker::ShouldCapitalize(ch, capitalizeNext);
[task 2024-08-05T19:26:37.100Z] 19:26:37 INFO - | ^~~~~~~~~~~~~
[task 2024-08-05T19:26:37.101Z] 19:26:37 INFO - | ULineBreak
[task 2024-08-05T19:26:37.101Z] 19:26:37 INFO - /builds/worker/workspace/obj-build/dist/include/unicode/uchar.h:2451:3: note: 'ULineBreak' declared here
[task 2024-08-05T19:26:37.101Z] 19:26:37 INFO - 2451 | } ULineBreak;
[task 2024-08-05T19:26:37.101Z] 19:26:37 INFO - | ^
[task 2024-08-05T19:26:37.101Z] 19:26:37 ERROR - /builds/worker/checkouts/gecko/layout/generic/nsTextRunTransformations.cpp:709:43: error: no member named 'ShouldCapitalize' in 'ULineBreak'
[task 2024-08-05T19:26:37.101Z] 19:26:37 INFO - 709 | doCapitalize = nsLineBreaker::ShouldCapitalize(ch, capitalizeNext);
[task 2024-08-05T19:26:37.102Z] 19:26:37 INFO - | ~~~~~~~~~~~~~~~^
[task 2024-08-05T19:26:37.102Z] 19:26:37 INFO - 2 errors generated.
[task 2024-08-05T19:26:37.102Z] 19:26:37 ERROR - gmake[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:674: nsTextRunTransformations.o] Error 1
[task 2024-08-05T19:26:37.102Z] 19:26:37 INFO - gmake[4]: Leaving directory '/builds/worker/workspace/obj-build/layout/generic'
[task 2024-08-05T19:26:37.102Z] 19:26:37 INFO - gmake[4]: Target 'target-objects' not remade because of errors.
[task 2024-08-05T19:26:37.102Z] 19:26:37 ERROR - gmake[3]: *** [/builds/worker/checkouts/gecko/config/recurse.mk:72: layout/generic/target-objects] Error 2
[task 2024-08-05T19:26:37.102Z] 19:26:37 INFO - gmake[4]: Entering directory '/builds/worker/workspace/obj-build/netwerk/base'
[task 2024-08-05T19:26:37.102Z] 19:26:37 INFO - netwerk/base/nsAsyncRedirectVerifyHelper.o
[task 2024-08-05T19:26:37.102Z] 19:26:37 INFO - gmake[4]: Leaving directory '/builds/worker/workspace/obj-build/netwerk/base'
|
|
Assignee | |
Updated•3 months ago
|
|
|
||
Comment 14•3 months ago
|
||
|
||
Comment 15•3 months ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/6ec91944b8a3
https://hg.mozilla.org/mozilla-central/rev/1258a7cc64e7
|
||
Updated•3 months ago
|
|
|
Assignee | |
Comment 17•1 month ago
|
||
Original Revision: https://phabricator.services.mozilla.com/D218533
|
|
||
Updated•1 month ago
|
|
|
||
Comment 18•1 month ago
|
||
esr128 Uplift Approval Request
- User impact if declined: poor sizing/spacing in certain cases (e,g. <select>) when text-transform:capitalize is used
- Code covered by automated testing: yes
- Fix verified in Nightly: yes
- Needs manual QE test: no
- Steps to reproduce for manual QE testing: n/a
- Risk associated with taking this patch: low
- Explanation of risk level: Change only affects the text-transform:capitalize codepath, borrowing existing logic from line-breaking
- String changes made/needed: none
- Is Android affected?: yes
|
|
||
Updated•1 month ago
|
|
|
||
Comment 19•1 month ago
|
||
| uplift | ||
|
||
Updated•1 month ago
|
Description
•