Closed Bug 1521814 Opened 6 years ago Closed 6 years ago

Make RDM override meta viewport only when a new pref is set (defaulting to false).

Categories

(DevTools :: Responsive Design Mode, defect)

64 Branch
defect
Not set
normal

Tracking

(relnote-firefox 65+, firefox-esr60 unaffected, firefox65 verified, firefox66 verified)

VERIFIED FIXED
Firefox 66
Tracking Status
relnote-firefox --- 65+
firefox-esr60 --- unaffected
firefox65 --- verified
firefox66 --- verified

People

(Reporter: bradwerth, Assigned: bradwerth)

References

(Blocks 2 open bugs)

Details

(Keywords: regression, Whiteboard: [qa-triaged])

Attachments

(1 file, 1 obsolete file)

Bug 1290420 turned on our handling of the meta viewport whenever touch simulation was on in Responsive Design Mode. This created many UX problems that we didn't catch before the feature landed in release.

Let's retroactively put meta viewport handling behind a pref, default to false. Any tests that are requiring meta viewport handling can have the pref set to true.

Whoa, there already is a pref, dom.meta-viewport.enabled that seems to be named appropriately. It may be sufficient to apply this pref to the relevant code. If not, and we do need to make a new pref, we'll have to name the new pref even more precisely.

Summary: Put meta viewport handling behind a pref, default to false until RDM viewports are stable → Put meta viewport handling behind dom.meta-viewport.enabled pref, default to false on desktop until RDM viewports are stable
Blocks: 1521934
Assignee: nobody → bwerth
Summary: Put meta viewport handling behind dom.meta-viewport.enabled pref, default to false on desktop until RDM viewports are stable → Make RDM override meta viewport only when a new pref is set (defaulting to false).
Attachment #9038359 - Attachment is obsolete: true
Pushed by bwerth@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/864bf1afa755 Part 1: Make RDM meta viewport override dependent on a new pref, set to false for now. r=gl
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
Attachment #9038686 - Flags: approval-mozilla-beta?

I can't see the details for the beta uplift approval, so I'm repeating them (imperfectly) here:

Feature/Bug causing the regression: 1290420
User impact if declined: Responsive Design Mode will have various UX problems when displaying documents with a meta viewport tag.
Is this code covered by automated tests? Yes.
Has the fix been verified in Nightly? No.
Needs manual test from QE? No.
If yes, steps to reproduce:
List of other uplifts needed: None.
Risk to taking this patch: Medium.
Why is the change risky/not risky?
(and alternatives if risky): This patch puts the work of Bug 1290420 behind a pref and defaults that pref to false. If Nightly users have adjusted their content or workflow to compensate, those users will be surprised by this change. They will have to set the new pref to true to restore current behavior.
String changes made/needed: None.

Comment on attachment 9038686 [details]
Bug 1521814 Part 1: Make RDM meta viewport override dependent on a new pref, set to false for now. r=gl!

changes landing on central this week get merged automatically to beta for 66, with no manual uplifts.

Attachment #9038686 - Flags: approval-mozilla-beta?

Comment on attachment 9038686 [details]
Bug 1521814 Part 1: Make RDM meta viewport override dependent on a new pref, set to false for now. r=gl!

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

Bug 1290420

User impact if declined

Responsive Design Mode will have various UX problems when displaying documents with a meta viewport tag.

Is this code covered by automated tests?

Yes

Has the fix been verified in Nightly?

No

Needs manual test from QE?

No

If yes, steps to reproduce

List of other uplifts needed

None

Risk to taking this patch

Medium

Why is the change risky/not risky? (and alternatives if risky)

This patch puts the work of Bug 1290420 behind a pref and defaults that pref to false. If Nightly or Beta users have adjusted their content or workflow to compensate, those users will be surprised by this change. They will have to set the new pref to true to restore current behavior. However, there have been no release notes detailing the existence of this pref, so they won't know how to restore the current (problematic) behavior.

String changes made/needed

Attachment #9038686 - Flags: approval-mozilla-release?

If possible, we want this in Release 65 to circumvent the problems we've seen in Release 64. In my uplift request notes above, I misspoke. If RELEASE 64 users have adjusted to compensate for the problems with meta viewport handling in RDM, they will be surprised by this change.

Keywords: regression
Version: unspecified → 64 Branch

I'm a bit nervous about preffing off a feature we've already been shipping since 64. Do we have any idea of # of affected users? Is that something we should be relnoting?

Flags: needinfo?(bwerth)

I agree that this is unusual and Not a Good Precedent. It may be worth doing anyway, since the behavior with the feature on is pretty bad (Bug 1501665). If we do decide to uplift the change, then we should add a release note. Here's a proposal:

Added pref "devtools.responsive.metaViewport.enabled", defaulting to false, turning off support for <meta> viewport tags in Responsive Design Mode. This pref controls whether Firefox Responsive Design Mode will attempt to respect the size constraints in a <meta> viewport tag, when RDM Touch Simluation is toggled on. Our current implementation of meta viewports in RDM has problems with sizing the viewport correctly. If you want to restore the previous behavior, set this pref to true.

Flags: needinfo?(bwerth)

Comment on attachment 9038686 [details]
Bug 1521814 Part 1: Make RDM meta viewport override dependent on a new pref, set to false for now. r=gl!

[Triage Comment]
Not crazy about turning off an already-shipped feature in a dot release, but it sounds like the alternative is pretty bad from a UX standpoint too. Approved for 65.0.1.

Attachment #9038686 - Flags: approval-mozilla-release? → approval-mozilla-release+
Flags: qe-verify+

Hi guys, I noticed that since this functionality is something that can be controlled via a pref flip. Should we consider shipping this when ready via a staged rollout (slowly turn it on for release population) rather than on-by-default? https://wiki.mozilla.org/Release_Management/Process_coordination_for_handling_off-train_releases#Process:_Feature_rollout_via_Normandy

This is a good strategy to rollout slowly and cautiously versus on-by-default on release launch day.

Flags: needinfo?(gl)
Flags: needinfo?(bwerth)

(In reply to Ritu Kothari (:ritu) from comment #18)

Hi guys, I noticed that since this functionality is something that can be controlled via a pref flip. Should we consider shipping this when ready via a staged rollout (slowly turn it on for release population) rather than on-by-default? https://wiki.mozilla.org/Release_Management/Process_coordination_for_handling_off-train_releases#Process:_Feature_rollout_via_Normandy

This is a good strategy to rollout slowly and cautiously versus on-by-default on release launch day.

When we roll it out (again), we can certainly do it in a staged fashion.

Flags: needinfo?(bwerth)

Thanks Brad. Happy to help discuss the plan when you guys reach that point.

Whiteboard: [qa-triaged]

I reproduced this issue using Fx 66.0a1 (build ID:20190122215349) on Windows 10 x64.
I can confirm this issue is fixed, I verified using Fx 65.0.1 and Fx 66.0b7 on Windows 10 x64, Ubuntu 16.04 LTS and macOs 10.13.

Status: RESOLVED → VERIFIED
Flags: qe-verify+

Clearing my needinfo since Brad already responded.

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

Attachment

General

Created:
Updated:
Size: