Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify the dropdownToolbarButton-select width computation #15552

Merged

Conversation

Snuffleupagus
Copy link
Collaborator

The way that we set the width of the dropdownToolbarButton-select is very old, and despite some improvements over the years this is still somewhat hacky. In particular, note how we're assigning the select-element a larger width than its containing dropdownToolbarButton-element. This was done to prevent displaying two separate icons, i.e. the native and the PDF.js one, since it's the only way to handle this in older browsers (particularly Internet Explorer).

Given the currently supported browsers, there's however a better solution available: use appearance: none; to disable native styling of the select-element. According to MDN, this is supported in all reasonably modern browsers. This way we're able to simplify both the CSS rules and the JS-code that's used to adjust the dropdownToolbarButton width in a localization aware way.

The way that we set the width of the `dropdownToolbarButton`-select is very old, and despite some improvements over the years this is still somewhat hacky.
In particular, note how we're assigning the select-element a larger width than its containing `dropdownToolbarButton`-element. This was done to prevent displaying *two* separate icons, i.e. the native and the PDF.js one, since it's the only way to handle this in older browsers (particularly Internet Explorer).

Given the currently supported browsers, there's however a better solution available: use `appearance: none;` to disable native styling of the select-element. [According to MDN](https://developer.mozilla.org/en-US/docs/Web/CSS/appearance#browser_compatibility), this is supported in all reasonably modern browsers.
This way we're able to simplify both the CSS rules and the JS-code that's used to adjust the `dropdownToolbarButton` width in a localization aware way.
@Snuffleupagus
Copy link
Collaborator Author

/botio-linux preview

@pdfjsbot
Copy link

pdfjsbot commented Oct 7, 2022

From: Bot.io (Linux m4)


Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/370e9afb4a9251c/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Oct 7, 2022

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/370e9afb4a9251c/output.txt

Total script time: 2.03 mins

Published

This attribute is overridden by the explicit `width` that's specified in the CSS rules, hence we can remove one more small piece of very old code; see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#size
@timvandermeij timvandermeij merged commit bf6e287 into mozilla:master Oct 9, 2022
@timvandermeij
Copy link
Contributor

Thank you for simplifying this!

@Snuffleupagus Snuffleupagus deleted the simplify-dropdownToolbarButton branch October 10, 2022 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants