Refreshing page while Web Developer Inspector is open never remembers scroll position

RESOLVED FIXED in Firefox 57

Status

defect
P2
normal
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: yoni, Assigned: zer0)

Tracking

(Depends on 1 bug)

54 Branch
Firefox 57
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(1 attachment)

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170628075643

Steps to reproduce:

Open any page, long or short, scroll down, open Web Developer console via F12 or keyboard shortcut (eg Ctrl Shift C for inspector).  Scroll down any length. Refresh via button or F5. 


Actual results:

Page always reloads at top, forgetting position in page.


Expected results:

Page should remember position, very awful for workflow.  When console is closed and page is refreshed (button or F5) Firefox remembers position perfectly and returns to same place.
Problem happens whether console is docked at bottom, sidebar or popped out into new window - upon reload will never remember page position. Tried on brand new FireFox, brand new profile - same results.
Component: Untriaged → Developer Tools: Console
I am reproducing on Nightly 56.0a1 (2017-07-18), but only if I open the inspector. If I only open the console (Cmd + alt + K) and scroll in it, the scroll position of the page is still kept on reload.
Component: Developer Tools: Console → Developer Tools: Inspector
Priority: -- → P3
Summary: Refreshing page while Web Developer console is open never remembers scroll position → Refreshing page while Web Developer Inspector is open never remembers scroll position
Just tried the very latest Nightly - 56.0a1 (2017-07-26) (64-bit) - can confirm exact same problem is still happening! I know we're not supposed to complain, just report bugs but this is incredibly aggravating and makes me need to developer work in Chrome, which I have avoided until now.
Hi yoni, I understand your frustration :) we need to fix this.

Matteo, pbro was saying to me that this can be due to Highlighters, does it look familiar to you ?
Flags: needinfo?(zer0)
I'm having the same problem. It is really annoying when building on a website.
I'm having the same issue with Firefox Developer Edition 56.0b5 (64-bit) on Ubuntu 17.04
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: P3 → P2
Just discussed with Matteo. It seems like bug 1341756 might be the cause of this bug. On reload, highlighters are removed, which makes the DOM of the page different which, in turn, prevents the auto-scroll feature from working after reload.
(In reply to Patrick Brosset <:pbro> from comment #7)
> Just discussed with Matteo. It seems like bug 1341756 might be the cause of
> this bug. On reload, highlighters are removed, which makes the DOM of the
> page different which, in turn, prevents the auto-scroll feature from working
> after reload.
Confirmed. Removing the fix for bug 1341756 fixes this bug.
Is there a way to remove the fix and/or download a different nightly/previous build that fixes this bug? THanks.
I did some tests, we could basically revert the changes of bug 1341756, basically removing this line:

http://searchfox.org/mozilla-central/source/devtools/server/actors/highlighters/utils/markup.js#311

I thought that doing so it would cause again the behavior fixed by such bug; but *apparently* not. Visually, everything works fine.
However, it would make the RDM causing the same issue of bug 1396690, that currently seems to happen specifically when we drag & drop a tab (the swapFrameLoaders is involved here too).

To be more precise, entering and quitting RDM mode, would add new highlighters markup every time, even if – compare to bug 1341756 – the old ones would be hidden and inactive.

I would suggest to revert bug 1341756 here, add a unit test to check that the scroll is working as intended for any future regression, and fixes the duplicated markup in bug 1396690.
Flags: needinfo?(zer0)
Assignee: nobody → zer0
Comment on attachment 8904468 [details]
Bug 1382341 - removed `this._remove()` from `onWindowReady`;

https://reviewboard.mozilla.org/r/176316/#review181218

Feels a bit weird just removing a prior bug fix, but if the fix was useless, then great!

::: devtools/client/inspector/test/doc_inspector_highlighter_scroll.html:7
(Diff revision 1)
> +<html>
> +  <head>
> +    <meta charset="utf-8">
> +    <style>
> +      p {
> +        height: 3000px;

3000px is probably ok for all screens we test on, but why not use 200vh instead? At least you're garanteed to be future-proof. You never know, maybe one day we add a test VM that has a crazy long screen size :)
Attachment #8904468 - Flags: review?(pbrosset) → review+
(In reply to Patrick Brosset <:pbro> from comment #12)

> Feels a bit weird just removing a prior bug fix, but if the fix was useless,
> then great!

It wasn't. It is now! Apparently before `onpagehide` it wasn't emit properly during the `swapFrameLoaders` therefore we had the issue shown here: https://bug1341756.bmoattachments.org/attachment.cgi?id=8840059

But I'm not able to reproduce it anymore (with this new patch applied), and the only downside is that we introduced the issue described in bug 1396690 (only when we enter / quitting RDM); but I think it's better having this than the document that lost the previous scroll – also, we're going to address that in bug 1396690 anyway.

> ::: devtools/client/inspector/test/doc_inspector_highlighter_scroll.html:7
> (Diff revision 1)
> > +<html>
> > +  <head>
> > +    <meta charset="utf-8">
> > +    <style>
> > +      p {
> > +        height: 3000px;
> 
> 3000px is probably ok for all screens we test on, but why not use 200vh
> instead? At least you're garanteed to be future-proof. You never know, maybe
> one day we add a test VM that has a crazy long screen size :)

Indeed! :)
Pushed by mferretti@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd3069bc00a3
removed `this._remove()` from `onWindowReady`; r=pbro
Here the try build:

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

The oranges doesn't seems related. I fixed the ES error before pushing to inbound (even if mozreview doesn't seems to show the line at the end of the file).
https://hg.mozilla.org/mozilla-central/rev/dd3069bc00a3
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
I have reproduced this bug according to (2017-07-19)

Fixing bug is verified on Latest Nightly--
Build ID    :20170915220136
User Agent  :Mozilla/5.0 (Windows NT 6.1; rv:57.0) Gecko/20100101 Firefox/57.0

Tested OS-- WIndows7 32bit
QA Whiteboard: [testday-20170915]
Product: Firefox → DevTools
Depends on: 1470642
You need to log in before you can comment on or make changes to this bug.