mozilla / bedrock

Making mozilla.org awesome, one pebble at a time
https://www.mozilla.org
Mozilla Public License 2.0
1.18k stars 920 forks source link

Styles should not use the non-standard `zoom` property #11326

Open eemeli opened 2 years ago

eemeli commented 2 years ago

Description

When a page from mozilla.org is rendered in Firefox, the following warning is printed in the console:

This page uses the non standard property “zoom”. Consider using calc() in the relevant property values, or using “transform” along with “transform-origin: 0 0”.

This is a blocker for bug 1501881, as a page from mozilla.org is rendered during first startup and the warning string source is currently in a .properties file.

Steps to reproduce

  1. Open https://www.mozilla.org/ in Firefox 71 or later (the warning was added in bug 1582374)
  2. Open the console and observe: Screenshot 2022-03-18 at 11 37 09

Expected result

No such warning should be printed.

Actual result

A warning was printed.

Environment

macOS Monterey, Firefox Nightly (100)

maureenlholland commented 2 years ago

Fix will have to override Protocol or come through an update to Protocol as it appears our main use of zoom is in the clearfix mixin cc: @craigcook https://github.com/mozilla/protocol/blob/557f04e6252ee327760b2111d03d88a8a53d6f30/src/assets/sass/protocol/includes/_mixins.scss#L19

components and templates using clearfix: https://github.com/mozilla/protocol/search?q=clearfix

there are also a few stray references directly in Bedrock for careers Protocol CSS and IE navigation https://github.com/mozilla/bedrock/search?l=SCSS&q=zoom

eemeli commented 2 years ago

From what I can tell, the only role of a zoom: 1 in a clearfix is to trigger hasLayout in IE < 8. As that's not supported by Protocol's browserslist, it should be fine to just drop it?

craigcook commented 2 years ago

Yeah, zoom is strictly used for hasLayout in old IE. We can remove it and put it in the stylesheet that is served only to IE in conditional comments.