1352814 - [e10s] Blank chrome page opens when restoring tab opened from Cmd + Click and RDM was used

Status: VERIFIED FIXED

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

Description

[684sigma]

I have a problem with Firefox Beta 52, Beta 53. It also happens in Nightly 55. Doesn't happen in ESR 45
When I restore some tabs that were or are in RDM, a blank page opens instead.

1. Ctrl+Shift+click this link: https://bugzilla.mozilla.org
2. Open RDM (then you can close it, doesn't matter).
3. Close the tab. Restore the tab.

Result: Blank chrome://devtools/content/responsive.html/index.xhtml page opens
Expected: https://bugzilla.mozilla.org should open

Pay special attention: it happens even when closing tab with RDM disabled.

Comment 1

[Loic]

It appears only with the new RDM available when e10s is enabled.

Comment 2

[Loic]

jryans, you fixed a similar issue in bug 1319596, could you check this one when you get some time, please.

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

I'll take a look.  Locally, it seems to affect any links you open using Ctrl / Cmd + Click.

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Actually, it appears even Ctrl / Cmd + Click isn't required:

STR:

1. Open a new tab, go to some page
2. Open RDM in that tab, but don't navigate anywhere else
3. Close the tab
4. Try to restore the closed tab

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

If you _do_ navigate to some new page after opening RDM, this happens to workaround the bug and full history for the tab is preserved.

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3202855c50126620690123f052f76dc4a1d23739

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1352814 - Force session history off for RDM container.

In bug 1313933, we removed the session store black magic that RDM used to do in
order to hide the container tab.

Unfortunately, that fix appears to have been imperfect.  Session store has a
fallback path that can still record the current URL, causing the container URL
to be recorded anyway, even though we asked nicely to please not do that.

In this change, we try a fresh approach of wedging the session history listener
for the container tab so it can't record anything.  This avoids the racy
approach that was used before bug 1313933 while still appearing to block the
container URL from being recorded.

Review commit: https://reviewboard.mozilla.org/r/131332/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/131332/

Comment 8

[Alexandre Poirot [:ochameau]]

Comment on attachment 8859312 [details]
Bug 1352814 - Force session history off for RDM container.

https://reviewboard.mozilla.org/r/131332/#review134434

In my testing I had to use Ctrl+Click to reproduce.
It fixes the reporter issue, but we should start considering integrating with /browser/ codebase as this project becomes the default responsive design and stop having these pile of hacks.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53575b5ff0a5
Force session history off for RDM container. r=ochameau

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/53575b5ff0a5

Comment 11

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8859312 [details]
Bug 1352814 - Force session history off for RDM container.

Approval Request Comment
[Feature/Bug causing the regression]: Most likely regressed by some clean up in bug 1313933.
[User impact if declined]: If declined, session history can be lost of tabs where Responsive Design Mode is opened.
[Is this code covered by automated tests?]: Yes, the patch adds a new test for this.
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Yes, see comment 0 and / or comment 4.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Only affects Responsive Design Mode
[String changes made/needed]: None

Comment 12

[Gerry Chang [:gchang]]

Hi Florin,
could you help find someone to verify if this issue was fixed as expected?

Comment 13

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Since it's RDM related - Mihai, can you verify this when you have some time?

Comment 14

[Mihai Boldan, Desktop QA [:mboldan]]

I managed to reproduce the initial issue on Firefox 53.0a1 (2016-12-20), under Windows 10x64.

The issue is no longer reproducible on Firefox 55.0a1 (2017-04-20).
Tests were performed using the STR from Comment 0 and from Comment 4, under Windows 10x64, Mac OS X 10.10.4 and under Ubuntu 16.04x64. 

I'm not removing the qe-verify + flag until I'll verify the issue also on Firefox Beta build.

Comment 15

[Gerry Chang [:gchang]]

Comment on attachment 8859312 [details]
Bug 1352814 - Force session history off for RDM container.

Fix a regression and was verified by QA. Beta54+. Should be in 54 beta 2.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

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

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Backed out for various RDM test failures.
https://hg.mozilla.org/releases/mozilla-beta/rev/953ebf1c32152dca8b633aa8d5a3fe3e684a56c5

https://treeherder.mozilla.org/logviewer.html#?job_id=93839127&repo=mozilla-beta

Comment 18

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a90f35b1934c1085c653f7bce9811f737505854

Comment 19

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b422a14b0f430fbeb8123c73ebaf67693a731054

Comment 20

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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

Comment 21

[Emil Ghitta, QA [:emilghitta]]

I’ve reproduced the issue described in comment 0 using Firefox 53.0 (Build Id:20170413192749) and verified that the issue is not reproducible anymore using Firefox 54.0b2 (BuildId:20170424145525)on Windows 10 64bit,macOS 10.11.6 and Ubuntu 14.04 64bit.