Closed
Bug 1506751
Opened 4 years ago
Closed 4 years ago
Firefox developer tools - css rules in devtools pane not updating after page refresh.
Categories
(DevTools :: Inspector: Rules, defect, P1)
Tracking
(firefox-esr60 unaffected, firefox63 wontfix, firefox64 wontfix, firefox65 verified)
VERIFIED
FIXED
Firefox 65
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox63 | --- | wontfix |
firefox64 | --- | wontfix |
firefox65 | --- | verified |
People
(Reporter: rngelliot, Assigned: rcaliman)
Details
(Whiteboard: [dt-q])
Attachments
(2 files)
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.77 Safari/537.36 Steps to reproduce: Load page Select element with media query max-width rule Change screen width to trigger css border color changes on element Refreshed using Ctrl + F5 (and also ctrl + R) Video of problem here: https://i.redd.it/usia2q4c6mx11.gif Tested also with other rules in media query. Same bug. Tested on clean install of Firefox, Developer edition and Portable edition, and Nightly v65.x Windows 7, 64bit, FF v63.0.1 Actual results: Before refresh style updated correctly in devtools rules pane After refresh style doesn't change to reflect border color change Note: the style on the page is changing correctly, but the rule in the devtools pane is not after a page refresh. Closing and reopening tab/browser restores correct behavior. It's the refresh that causes the bug. Expected results: After page refresh rules should update in rules pane to reflect css style changes based on media query
Summary: Firefox developer tools - rules in media query not updating after page refresh. → Firefox developer tools - css rules in devtools pane not updating after page refresh.
Comment 1•4 years ago
|
||
Hey nuwanda, Can you please also attach the page you loaded? Adding to a Component given that the reporter reproduced it on a clean profile. There is also a bug report that looks similar to this, not sure if it is the same tho: Bug 1108502
Component: Untriaged → CSS Rules Inspector
Flags: needinfo?(rngelliot)
Product: Firefox → DevTools
Updated•4 years ago
|
Whiteboard: [dt-q]
To reproduce bug: -Load index2.html -In devtools, select container-1 -Resize window either side of 500px width to trigger media query -container-1 border will change red/blue and rule will update in devtools styles pane -Refresh page (ctrl + F5, or ctrl + R) and repeat window resize to trigger style changes -Border color will red/blue on page but rule will NOT change in devtools styles pane -Close and reopen tab -devtools rules now update correctly
Flags: needinfo?(rngelliot)
(In reply to Timea Babos from comment #1) > Hey nuwanda, > > Can you please also attach the page you loaded? > Adding to a Component given that the reporter reproduced it on a clean > profile. > > There is also a bug report that looks similar to this, not sure if it is the > same tho: Bug 1108502 Attached. Yes, it seems similar to 1108502 but that report involved an extension. This bug is with clean installs of all three FF versions and Nightly.
Comment 4•4 years ago
|
||
Reproduced on latest Nightly 65.0a1 (2018-11-19), Beta 64.0b10 and Release 63.0.3. on Windows 10 x64. Thanks again for the test case and steps!
Status: UNCONFIRMED → NEW
status-firefox63:
--- → affected
status-firefox64:
--- → affected
status-firefox65:
--- → affected
Ever confirmed: true
(In reply to Timea Babos from comment #4) > Reproduced on latest Nightly 65.0a1 (2018-11-19), Beta 64.0b10 and Release > 63.0.3. on Windows 10 x64. > Thanks again for the test case and steps! No worries. It's been driving me nuts. This is a pretty major bug in my opinion as it goes directly to having trust in the devtools to show accurate information. I've been using Chrome for my media queries testing, which is pretty inconvenient to say the least. I look forward to a fix so I can come back to FF. Here's another page I used for testing which will be easier to share: http://www.webdesignerwall.com/demo/media-queries/
Comment 6•4 years ago
|
||
Thanks for filing. This is indeed a very frustrating issue. I think I found the root cause for it. When DevTools starts, we listen for window resize so we can correctly show new rules when media queries kick in. The problem is that after a page refresh (or navigation), we don't listen again for resize in the new window. So we should listen for target navigation events and use those to start listening again to resize after the new window has been navigated to. This should be done in the WindowResizeObserver class, in the /devtools/server/actors/reflow.js file. Specifically, the targetActor constructor argument should be used to add an event listener like this: targetActor.on("navigate", this.onNavigate); and then the onNavigate callback should be used to stop and restart the window resize listener, so it points to the right window.
Priority: -- → P1
Assignee | ||
Updated•4 years ago
|
Assignee: nobody → rcaliman
Status: NEW → ASSIGNED
Assignee | ||
Comment 7•4 years ago
|
||
Pushed by rcaliman@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0fa44cb3f4b6 Re-assign listeners for Observable instances that monitor TargetActor windows on navigate. r=pbro
Assignee | ||
Comment 9•4 years ago
|
||
Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ca435dba55048d92dbd71645f7d466e231123aa
Comment 10•4 years ago
|
||
Backed out for xpcshell failures on reflow.js Backout link: https://hg.mozilla.org/integration/autoland/rev/f85343663811990b19cecb27b7c0beba0448309a Push link: https://hg.mozilla.org/integration/autoland/rev/0fa44cb3f4b686913a397c6297c26d5f9c64bcb9 Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=213560705&repo=autoland&lineNumber=7666
Flags: needinfo?(rcaliman)
Assignee | ||
Comment 11•4 years ago
|
||
Fixed issue. Ran successfully on try for xpcshell tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b41930a92ff2aff6b0af875592bffcc44b76ec47
Flags: needinfo?(rcaliman)
Comment 12•4 years ago
|
||
Pushed by rcaliman@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/507a08aaccd5 Re-assign listeners for Observable instances that monitor TargetActor windows on navigate. r=pbro
Comment 13•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/507a08aaccd5
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Reporter | ||
Comment 14•4 years ago
|
||
Quick work. This will be in the next Nightly? Currently I'm on 65.0a1.
Comment 15•4 years ago
|
||
Verified - Fixed on Windows 10 x64, Mac OS 10.13 and Ubuntu 16.04. on the latest Nightly 65.0a1 (2018-11-25) (64-bit). Nuwanda, can you please check if it is also fixed on your side? Please make sure to update to the latest Nightly. Thanks!
Flags: needinfo?(rngelliot)
Comment 16•4 years ago
|
||
Is this something we should nominate for Beta approval ahead of next week's RC builds?
Reporter | ||
Comment 17•4 years ago
|
||
(In reply to Timea Babos from comment #15) > Nuwanda, can you please check if it is also fixed on your side? Please make > sure to update to the latest Nightly. Thanks! Verified FIXED on Nightly 65.0a1. Rules in media queries now update in devtools style pane after refresh.
Status: RESOLVED → VERIFIED
Flags: needinfo?(rngelliot)
Assignee | ||
Comment 18•4 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #16) > Is this something we should nominate for Beta approval ahead of next week's > RC builds? Yes, thank you for the reminder! This is an issue in current Beta and it will slip to Stable unless uplifted. Requesting that now.
Flags: needinfo?(rcaliman)
Assignee | ||
Comment 19•4 years ago
|
||
Comment on attachment 9026721 [details] Bug 1506751 - Re-assign listeners for Observable instances that monitor TargetActor windows on navigate. r=pbro [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: Bug 1506751 User impact if declined: Media queries in the Inspector Rules view do not get updated after the user refreshes the page. This is a serious issue because it makes users lose trust in the accuracy of the tool. Is this code covered by automated tests?: Yes Has the fix been verified in Nightly?: Yes Needs manual test from QE?: Yes If yes, steps to reproduce: Yes, see instructions in this comment: https://bugzilla.mozilla.org/show_bug.cgi?id=1506751#c2 List of other uplifts needed: None Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): The fix ensures the listeners for media query updates are re-attached after page refresh. This should not impact other parts. String changes made/needed: None
Attachment #9026721 -
Flags: approval-mozilla-beta?
Comment 20•4 years ago
|
||
(In reply to Razvan Caliman [:rcaliman] from comment #19) > Feature/Bug causing the regression: Bug 1506751 The intent here is to ask when the issue was introduced. Not which bug this is. And since I'm here... as we're late in the beta cycle, and this is already present in 63, is there a particular reason letting the fix ride the trains to 65 would be bad?
Flags: needinfo?(rcaliman)
Reporter | ||
Comment 21•4 years ago
|
||
(In reply to Julien Cristau [:jcristau] from comment #20) > (In reply to Razvan Caliman [:rcaliman] from comment #19) > And since I'm here... as we're late in the beta cycle, and this is already > present in 63, is there a particular reason letting the fix ride the trains > to 65 would be bad? "This is a serious issue because it makes users lose trust in the accuracy of the tool." Isn't this enough to move it along ASAP? I filed the bug and have been using Chrome for my development due to the subsequent concerns about the accuracy of Firefox devtools. Some bugs are merely inconvenient or annoying but this goes directly to the viability of Firefox as a trusted tool for developers.
Comment 22•4 years ago
|
||
Comment on attachment 9026721 [details] Bug 1506751 - Re-assign listeners for Observable instances that monitor TargetActor windows on navigate. r=pbro Anyway, 64 is on release now, so moving the nomination flag accordingly.
Attachment #9026721 -
Flags: approval-mozilla-beta? → approval-mozilla-release?
status-firefox-esr60:
--- → unaffected
Assignee | ||
Comment 23•4 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #22) > Comment on attachment 9026721 [details] > Bug 1506751 - Re-assign listeners for Observable instances that monitor > TargetActor windows on navigate. r=pbro > > Anyway, 64 is on release now, so moving the nomination flag accordingly. Sorry for dropping the ball on this. Given how late in the cycle it is, I think it's fine to just let the patch ride the trains for last 2-3 weeks before organically reaching Firefox release. I'll pay more attention next time to pinpoint the regression bug in the uplift request form.
Flags: needinfo?(rcaliman)
Assignee | ||
Comment 24•4 years ago
|
||
Comment on attachment 9026721 [details] Bug 1506751 - Re-assign listeners for Observable instances that monitor TargetActor windows on navigate. r=pbro This patch will reach Firefox release soon anyway; withdrawing uplift request.
Attachment #9026721 -
Flags: approval-mozilla-release?
Updated•4 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•