Bug 1521814
| Summary: | Make RDM override meta viewport only when a new pref is set (defaulting to false). | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Components] DevTools | Reporter: | Brad Werth [:bradwerth] <bwerth> | ||||||
| Component: | Responsive Design Mode | Assignee: | Brad Werth [:bradwerth] <bwerth> | ||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | -- | CC: | bogdan.maris, botond, cristian.comorasu, gl, jcristau, mbalfanz, mmucci, rkothari, ryanvm | ||||||
| Version: | 64 Branch | Keywords: | regression | ||||||
| Target Milestone: | Firefox 66 | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Whiteboard: | [qa-triaged] | ||||||||
| relnote-firefox: | 65+ | Crash Signature: | |||||||
| Last Resolved: | 2019-01-24 21:50:17 | User Story: | |||||||
| QA Whiteboard: | Iteration: | --- | |||||||
| Points: | --- | Has Regression Range: | --- | ||||||
| Has STR: | --- | tracking-geckoview66: | --- | ||||||
| status-geckoview66: | --- | tracking-firefox-esr60: | --- | ||||||
| status-firefox-esr60: | unaffected | status-firefox65: | verified | ||||||
| tracking-firefox66: | --- | status-firefox66: | verified | ||||||
| tracking-firefox67: | --- | status-firefox67: | --- | ||||||
| tracking-firefox68: | --- | status-firefox68: | --- | ||||||
| Fission Milestone: | --- | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 1411650, 1521921, 1521934, 1290420 | ||||||||
| Attachments: |
|
||||||||
|
Description
Brad Werth [:bradwerth]
2019-01-22 08:55:54 PST
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. Created attachment 9038359 [details] Bug 1521814 Part 1: Make the existing dom.meta-viewport-enabled pref act as an off-switch for desktop meta viewport. Created attachment 9038686 [details] Bug 1521814 Part 1: Make RDM meta viewport override dependent on a new pref, set to false for now. r=gl! 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 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. 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 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. 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? 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. 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. 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. (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. Thanks Brad. Happy to help discuss the plan when you guys reach that point. 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. Clearing my needinfo since Brad already responded. |
