1500921 - Wrong behavior of "Page Style" menu in Responsive Design mode

Status: VERIFIED FIXED

(DevTools :: Responsive Design Mode, defect, P3)

Description

[Kuba Orlik]

Attachment: screenshot.png

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:63.0) Gecko/20100101 Firefox/63.0

Steps to reproduce:

Open a website in reponsive design mode, and then pick "View -> Page Style -> No Style". It applies the change to the browser UI, not to the webpage

Comment 1

[Peter Magyari (Desktop QA)]

Hi,

I haven't been able to reproduce this issue using Firefox Release 63.0/Firefox Beta 64.0b3/Firefox Nightly 65.0a1.

Could you test if the issue is reproducible in safe mode, here is a link that can help you do that:
https://support.mozilla.org/en-US/kb/troubleshoot-firefox-issues-using-safe-mode

If the issue still occurs please try using a new profile, you can find the steps here:
https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles?redirectlocale=en-US&redirectslug=Managing-profiles#w_starting-the-profile-manager

Thank you!

Comment 2

[Peter Magyari (Desktop QA)]

Due to the lack of response from the reporter I'm marking it as resolved:incomplete. 
If you would like to add more information that could help us reproduce this issue please feel free to reopen it.

Comment 3

[Kuba Orlik]

I confirm this issue is affecting me on 64.0b8 (64-bit) in safe mode as well as when using a new profile. Not sure how I can reopen the issue.

Comment 4

[Daniel Cicas [:dcicas], Release QA]

I can still reproduce this issue under the following OS's: Windows 10 x32, mac OS 10.13.6, Ubuntu 16.04 and Ubuntu 18.04. The affected builds are: 65.0a1, 64.0b14, 63.0.1 and 60.0.2esr.

Comment 5

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

Thank you, Daniel!

Comment 6

[Peter Magyari (Desktop QA)]

Thank you for taking the time to report this issue!

Comment 7

[Patrick Brosset <:pbro>]

Thanks for filing. 
This is probably another RDM tunnel-related problem, whereby RDM (Responsive Design Mode) should forward the page style action to the inner viewport through its tunnel, rather than letting the action apply to itself.

Comment 8

[Patrick Brosset <:pbro>]

I was looking a bit into this and went far enough to understand the solution, so I'll push a patch here.

Comment 9

[Patrick Brosset <:pbro>]

Attachment: Bug 1500921 - Tunnel the pagestyle message through RDM; r?gl!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c9fa642e9e3a
Tunnel the pagestyle message through RDM; r=gl

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/c9fa642e9e3a

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta approval when you get a chance.

Comment 13

[Patrick Brosset <:pbro>]

Comment on attachment 9030714 [details]
Bug 1500921 - Tunnel the pagestyle message through RDM; r?gl!

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1305769

User impact if declined: This isn't a new regression. The new RDM was implemented 3 years ago, and enabled for real 2 years ago (in bug 1305769).
This limitation with the Page Style menu has existed since then.

If we decline this fix on beta, then people who use the Responsive Design Mode and who try to select styles from the Page Style menu won't see these changes affect the page displayed in RDM. Instead, the changes will affect the RDM UI itself.
In particular, they won't be able to turn off styles on the current page.

This probably represents a small fraction of Firefox users though.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): RDM works in a way where it creates a sub-iframe inside the current tab, and it relays all actions from the browser to the nested iframe (like when you navigate to a different page, instead of the new page replacing RDM, the message is relayed to the inside of the iframe).
So this fix simply adds handling for the page style messages, so that they, too, are relayed to the nested iframe instead of applying to the RDM UI.

String changes made/needed:

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9030714 [details]
Bug 1500921 - Tunnel the pagestyle message through RDM; r?gl!

[Triage Comment]
Allows users who use the Responsive Design Mode and who try to select styles from the Page Style menu to see these changes affect the page displayed in RDM. Thanks for including an automated test with this, though I still think some extra QA verification would be good here as well. Approved for 65.0b6.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f77371a128ae

Comment 16

[Catalin Sasca, Desktop QA [:csasca]]

I successfully reproduced the issue on Firefox Nightly 64.0a1 (2018-10-22) under Windows 10 (x64) using the STR from Comment 0.

The issue is no longer reproducible on Firefox Beta 65.0b6 (20181220042023) and Firefox Nightly 66.0a1 (2018-12-19). Tests were performed under Windows 10 (x64), Ubuntu 16.04 (x64) and macOS 10.12.