1599324 - Popup doesn't appear in the center, can't scroll to click the buttons

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Karl Dubost💡 :karlcow]

This was reported by Panos and this is a regression because of the zoom hack attempt.

Basically the issue is with this part of the CSS rule. I cut the rest for brevity.

.modal.modal--show .modal-container {

	transform: translate(-50%,-50%);
	zoom: 1;
}

Applying the zoom with the transform cancels the effect of the transform: translate(-50%,-50%); and breaks the design on the site.

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1599324 - Disable layout.css.zoom-transform-hack.enabled for now. r=boris

People do like to use zoom along with transforms for legit reasons, which means
that we get this wrong.

This is unfortunate, as the whole point of the hack was fixing sites that only
used zoom without regressing sites that would do:

zoom: 0.5;
-moz-transform: scale(0.5);
transform-origin: 0 0;

So we're a bit stuck here. The only way to deal with the known regressions is
parsing zoom: 1 as invalid, which is insane, and it would probably cause other
compat issues... For now disable the pref.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9c10d8e6c8a4
Disable layout.css.zoom-transform-hack.enabled for now. r=boris

Comment 3

[Dorel Luca [:dluca]]

Backed out changeset 9c10d8e6c8a4 (bug 1599324) for Xpcshell failures in devtools/shared/tests/unit/test_css-properties-db.js. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=278288356&repo=autoland&lineNumber=2556

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=9c10d8e6c8a4f414c8ae8d5a792b4a7709a80bc7

Backout:
https://hg.mozilla.org/integration/autoland/rev/f1c654bdb2eb9fa76d961851e82a7b6b3a3f8e6b

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1599324 - Make one variant of BlobURLProtocolHandler::AddDataEntry infallible. r=baku

And clean-up surrounding code by removing useless namespace qualification.

It seems like the first call to Broadcast is useless? Or am I missing something?

Comment 5

[Phabricator Automation]

Comment on attachment 9112045 [details]
Bug 1599324 - Make one variant of BlobURLProtocolHandler::AddDataEntry infallible. r=baku

Revision D55000 was moved to bug 1599843. Setting attachment 9112045 [details] to obsolete.

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1599324 - Fix Servo_IsCssPropertyRecordedInUseCounter so that we also report disabled properties. r=boris

When zoom is disabled, we still count it, but with the current code the testing
function will throw instead of returning the right value, which means we'd fail
layout/style/test/test_use_counters.html.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3f942560109f
Fix Servo_IsCssPropertyRecordedInUseCounter so that we also report disabled properties. r=boris

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/461d07650b52
Disable layout.css.zoom-transform-hack.enabled for now. r=boris

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/3f942560109f
https://hg.mozilla.org/mozilla-central/rev/461d07650b52

Comment 10

[Daniel Holbert [:dholbert]]

From discussion at all-hands: we could address the scenario in comment 0 by ignoring zoom: 1, and maybe re-enable the pref...

Comment 11

[Emilio Cobos Álvarez [:emilio]]

Done in bug 1599072, which is basically this bug, and has some other relevant discussion.