1306892 - Firefox devtools load CSS twice if you open the style editor at least once

Status: RESOLVED FIXED

(DevTools :: Style Editor, defect, P2)

Description

[Rubén]

Attachment: Screenshot_2016-10-01_19-58-27.png

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID: 20160920074044

Steps to reproduce:

I Firefox, press F12 to open the Devtools click on Network Monitor and surf to any website (for instance https://www.bonnieradvocaten.nl/)  to measure it's performance using the Network Monitor.




Actual results:

Firefox fetches / "loads"  CSS file(s) that are needed to correctly show the webpage.  those are correctly fetched usually as one of the first requests ( right after the HTML is downloaded). Then  right after the page is fully loaded it will fetch / load that same CSS file, the one that was fetched as one of the first requests, again.  

The HTML of the web-page (i mentioned) is not the source of this issue it only includes the CSS file once as the first element to be loaded after the HTML is parsed. 

I'm attaching a screen-shot of the Network Monitor that clearly shows the problem. 

The Network Monitor itself points to a bunch of java-script files that are part of the Devtools as the cause of the .CSS file being loaded for a second time as can be seen in the aforementioned attachment. 

The problem presents itself with many different website and AFAIK was introduced with Firefox 49.

Also the problem is not present 100% percent of the time.


Expected results:

The .CSS file should have only been loaded once per the HTML of the web-page that is being visited.

Comment 1

[Rubén]

I would like to add that this is probably not related to Bug 1071613, since setting "devtools.styleeditor.source-maps-enabled: false"  does not fix the problem.

Comment 2

[Alice0775 White]

@Reporter

This seems to be fixed in Firefox Beta and later.
Could you test this with Firefox Beta50b3 [1]?

[1] https://www.mozilla.org/en-US/firefox/channel/#beta

Comment 3

[Alice0775 White]

I can reproduce on Nightly52.0a1.
Once open "Style editor" then css load twice.

Steps to reproduce:
1. Open Style editor and then click on Network Monitor
2. Load the URL https://www.bonnieradvocaten.nl/

Comment 4

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=711963e8daa312ae06409f8ab5c06612cb0b8f7b&tochange=1d657d984bed365b1c2bdc499532cf34c60b1edd

Suspect: Bug 1282660

Comment 5

[Rubén]

(In reply to Alice0775 White from comment #2)
> @Reporter
> 
> This seems to be fixed in Firefox Beta and later.
> Could you test this with Firefox Beta50b3 [1]?
> 
> [1] https://www.mozilla.org/en-US/firefox/channel/#beta

Besides Firefox 49 i also Firefox Developer Edition 51.0a2.  With the latest build (2016-10-01) i'm seeing the same problem. So i don't think it's fixed in Beta and later.

Comment 6

[Alice0775 White]

(In reply to buggyz from comment #5)
> (In reply to Alice0775 White from comment #2)
> > @Reporter
> > 
> > This seems to be fixed in Firefox Beta and later.
> > Could you test this with Firefox Beta50b3 [1]?
> > 
> > [1] https://www.mozilla.org/en-US/firefox/channel/#beta
> 
> Besides Firefox 49 i also Firefox Developer Edition 51.0a2.  With the latest
> build (2016-10-01) i'm seeing the same problem. So i don't think it's fixed
> in Beta and later.


Yep, see comment #3.
To reproduce the problem, Style editor must open at least once.

Comment 7

[Rubén]

This problem also appears when the Style Editor is not opened in any way.

Actions:
Today i started Firefox after it being fully closed, I then pressed F12, the Network Monitor appeared, > I continued browsing to https://www.bonnieradvocaten.nl/ without clicking or otherwise opening the Style Editor. 

Result:
The CSS file of the website was loaded twice just like before. Note that i did not click on the Style Editor nor did the style editor appear. 


The same goes for Firefox 51.0as2, the problem also appeard without the Style Editor being opened.

Comment 8

[Alice0775 White]

(In reply to buggyz from comment #7)
> This problem also appears when the Style Editor is not opened in any way.
> 
> Actions:
> Today i started Firefox after it being fully closed, I then pressed F12, the
> Network Monitor appeared, > I continued browsing to
> https://www.bonnieradvocaten.nl/ without clicking or otherwise opening the
> Style Editor. 
> 
> Result:
> The CSS file of the website was loaded twice just like before. Note that i
> did not click on the Style Editor nor did the style editor appear. 
> 
> 
> The same goes for Firefox 51.0as2, the problem also appeard without the
> Style Editor being opened.

umm, I cannot reproduce the problem without open Style Editer.

Comment 9

[Rubén]

(In reply to Alice0775 White from comment #8)
> (In reply to buggyz from comment #7)
> > This problem also appears when the Style Editor is not opened in any way.
> > 
> > Actions:
> > Today i started Firefox after it being fully closed, I then pressed F12, the
> > Network Monitor appeared, > I continued browsing to
> > https://www.bonnieradvocaten.nl/ without clicking or otherwise opening the
> > Style Editor. 
> > 
> > Result:
> > The CSS file of the website was loaded twice just like before. Note that i
> > did not click on the Style Editor nor did the style editor appear. 
> > 
> > 
> > The same goes for Firefox 51.0as2, the problem also appeared without the
> > Style Editor being opened.
> 
> umm, I cannot reproduce the problem without open Style Editor.

Yeah, the problem (without opening the style editor) isn't happening all the time. When i tested it with Firefox 49 and then with Firefox 51.0a2 this afternoon, i took extra care not to open the Style Editor, yet it did happen again with both browsers.

I’m not sure what else is triggering it but there seems to be more than one trigger.....

Comment 10

[Rubén]

Okay so i just found another way to make the Network Monitor load CSS twice: opening the web inspector at least once.

When i open the Network Monitor F12>Devtools and it instantly opens the Network Monitor everything is fine CSS is only loaded once.

 But, when i open the Network Monitor then click on "Inspector" then click on "Network Monitor" and load the website's CSS is loaded twice.

So opening the Style Editor or Web Inspector at least once causes CSS to be loaded twice 

I've tested both  Firefox 49.0 and on 51.0a2 (build 2016-10-12) and they both exhibit this behavior.

Comment 11

[Patrick Brosset <:pbro>]

(In reply to buggyz from comment #10)
> Okay so i just found another way to make the Network Monitor load CSS twice:
> opening the web inspector at least once.
> 
> When i open the Network Monitor F12>Devtools and it instantly opens the
> Network Monitor everything is fine CSS is only loaded once.
> 
>  But, when i open the Network Monitor then click on "Inspector" then click
> on "Network Monitor" and load the website's CSS is loaded twice.
> 
> So opening the Style Editor or Web Inspector at least once causes CSS to be
> loaded twice 
> 
> I've tested both  Firefox 49.0 and on 51.0a2 (build 2016-10-12) and they
> both exhibit this behavior.
Indeed, both the style editor and the inspector rely on the same back-end to get CSS style information. So opening any of these 2 panels before will have the same effect.
The back-end downloads the CSS file again to get the "authored" text to display in devtools. I don't remember if this is related to the devtools.styleeditor.source-maps-enabled setting or not. What I do remember is that if a stylesheet is set not to be cached, then the back-end has no other choices then to actually download it again from the server.

If this is the only problem you are seeing here, then this bug should be moved to the Style Editor bugzilla component and should block bug 1264624.

But comment 7 seems to say that this also happens without first opening the inspector or the style-editor.
Is this still the case? Or should we now consider this bug to only be a result of having opened those panels before?

Comment 12

[Rubén]

(In reply to Patrick Brosset <:pbro> from comment #11)
> (In reply to buggyz from comment #10)
> > Okay so i just found another way to make the Network Monitor load CSS twice:
> > opening the web inspector at least once.
> > 
> > When i open the Network Monitor F12>Devtools and it instantly opens the
> > Network Monitor everything is fine CSS is only loaded once.
> > 
> >  But, when i open the Network Monitor then click on "Inspector" then click
> > on "Network Monitor" and load the website's CSS is loaded twice.
> > 
> > So opening the Style Editor or Web Inspector at least once causes CSS to be
> > loaded twice 
> > 
> > I've tested both  Firefox 49.0 and on 51.0a2 (build 2016-10-12) and they
> > both exhibit this behavior.
> Indeed, both the style editor and the inspector rely on the same back-end to
> get CSS style information. So opening any of these 2 panels before will have
> the same effect.
> The back-end downloads the CSS file again to get the "authored" text to
> display in devtools. I don't remember if this is related to the
> devtools.styleeditor.source-maps-enabled setting or not. What I do remember
> is that if a stylesheet is set not to be cached, then the back-end has no
> other choices then to actually download it again from the server.
> 
> If this is the only problem you are seeing here, then this bug should be
> moved to the Style Editor bugzilla component and should block bug 1264624.
> 
> But comment 7 seems to say that this also happens without first opening the
> inspector or the style-editor.
> Is this still the case? Or should we now consider this bug to only be a
> result of having opened those panels before?

This should be considered to be only triggered by the the inspector and/or style-editor.

Comment 13

[Rubén]

I've just tried Firefox 53 (Nightly - build of 2016-11-14) it's also affected but i can't (yet) edit the tracking flags.

Comment 15

[Rubén]

[Tracking Requested - why for this release]:

Firefox 54.0a1 is also affected by this bug.

Comment 17

[Gerry Chang [:gchang]]

This is not a recent regression. It's been there for a while. Track Aurora54-. Mark 54 fix-optional but still happy to have the fix in 54.

Comment 18

[qdev labs]

Using Firefox for Mac 52.0.2 is still doing this.

I don't know which conditions have to be met, but on "normal" debugging it seems to load the CSS files multiple times (three !)

Comment 19

[maarthonderdelf]

For what it's worth, I observed this behaviour as well. However, the second time the CSS file was being loaded, the browser also loaded the main document a second time. Observe server logs while having browser cache switched off.

I kept the server waiting on a breakpoint in the main document, using Xdebug). During that time the browser showed on the network tab that it was waiting for de CSS file. The network tab did not show the second loading of the main document, but only the CSS file.

Using Firefox 53.0.3 (64-bit) for Ubuntu.

Comment 20

[Andy Mercer]

This is happening on Windows 10, Firefox 54.0.1 32-bit.

Comment 21

[Nathan Lenkowski [:nlenkowski]]

It looks like this bug still persists on Firefox 57. At first I thought the duplicate stylesheet request might be related to Webpack bundling, but alas it is this bug. The duplicate request disappears when opening the network panel directly and reloading the page. This stack overflow post saved me some time, https://stackoverflow.com/questions/41529386/multiple-get-requests-for-the-same-css-file-in-firefox-inspector.

Comment 22

[Richard Michael]

This still occurs on both FirefoxDeveloperEdition 58.0b10 and Firefox Quantum 57.0.1.

Is there any possibility this could be bumped in priority, please?

Given the comments here and on StackOverflow, this is probably causing quite a bit of lost time. I think few people will suspect a very mature browser such as Firefox to be making this mistake.  Rather, they will be more likely to suspect increasingly complicated asset build tools and dive in there to debug.  I just spent an hour trying to debug just a very simple static site, and it wasn't until I decided to quit and restart Firefox that I discovered it was probably the browser!  (My hint was that Chrome loaded just fine, when I eventually turned to it for comparison.)

Comment 23

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

Holding onto the sheet text would at least be a slight memory regression, but if that's a big deal, we could potentially do so only when the tools are open...

I was curious what Chrome does in this case... It appears they have a similar setup to us in that they don't appear to keep the sheet text in the platform, and they might fallback to fetch the text from the network just like we do. However, they do try one extra thing: they check if the tools have style text in memory[1] via the Resources tool. For us, I think the closest thing would be asking the Network Monitor if it saw the CSS request go by before we go to the last resort of fetching. If it does, then it can hand over the text as fetched.

I'll make an attempt at this.

[1]: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/inspector/InspectorStyleSheet.cpp?l=1825&rcl=c84e68d8561f2749b5b831aa57581349133f8800

Comment 24

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

Attachment: Bug 1306892 - Fetch stylesheets from network monitor.

If the toolbox is open when a stylesheet is loaded, the network monitor should
have recorded the response content.  When a tool asks for stylesheet text, try
asking the network monitor first before falling back to an extra fetch as a last
resort.

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

Comment 25

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

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

Comment 26

[Tom Tromey :tromey]

Comment on attachment 8940381 [details]
Bug 1306892 - Fetch stylesheets from network monitor.

https://reviewboard.mozilla.org/r/210646/#review216726

Thanks for the patch.  This is very nice.

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e188cb9fe8f5
Fetch stylesheets from network monitor. r=tromey

Comment 28

[Julian Descottes [:jdescottes]]

Comment on attachment 8940381 [details]
Bug 1306892 - Fetch stylesheets from network monitor.

https://reviewboard.mozilla.org/r/210646/#review216796

::: devtools/client/styleeditor/test/browser_styleeditor_fetch-from-cache.js:46
(Diff revision 1)
>        items.push(item);
>      }
>    }
>  
> -  is(items.length, 2,
> -     "Got two requests for doc_uncached.css after Style Editor was loaded.");
> +  is(items.length, 1,
> +     "Got one request for doc_uncached.css after Style Editor was loaded.");

Sorry about the flyby (really happy to see this getting fixed!), but I think this test might need updating. 

Initially this was opened for Bug 978688, because the StyleEditor was generating too much traffic. 

After this patch lands, if the netmonitor was opened, the style editor will not request the stylesheet a second time. 
But if the netmonitor was not opened, the style editor will still have to perform an additional call to request the stylesheet. 
In this second case, we should still verify that we use the cached version I guess?

The test now checks that we don't do extra requests if the netmonitor was opened, and consequently validates this bug. 
We need to update the name and jsdoc accordingly.

And we are now missing a test for Bug 978688. Which might be challenging to implement since we cannot use the netmonitor to inspect the requests.

Comment 29

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

Comment on attachment 8940381 [details]
Bug 1306892 - Fetch stylesheets from network monitor.

https://reviewboard.mozilla.org/r/210646/#review216796

> Sorry about the flyby (really happy to see this getting fixed!), but I think this test might need updating. 
> 
> Initially this was opened for Bug 978688, because the StyleEditor was generating too much traffic. 
> 
> After this patch lands, if the netmonitor was opened, the style editor will not request the stylesheet a second time. 
> But if the netmonitor was not opened, the style editor will still have to perform an additional call to request the stylesheet. 
> In this second case, we should still verify that we use the cached version I guess?
> 
> The test now checks that we don't do extra requests if the netmonitor was opened, and consequently validates this bug. 
> We need to update the name and jsdoc accordingly.
> 
> And we are now missing a test for Bug 978688. Which might be challenging to implement since we cannot use the netmonitor to inspect the requests.

As discussed on IRC, the network monitor is always listening on toolbox open these days.  So, there isn't really a "network monitor was not listening" case if we are running a test with our tools already open.

Anyway, we decided we should at least rename / update comments in the test.

Comment 30

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

Attachment: Bug 1306892 - Update style editor fetch test info.

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

Comment 31

[Julian Descottes [:jdescottes]]

Comment on attachment 8940793 [details]
Bug 1306892 - Update style editor fetch test info.

https://reviewboard.mozilla.org/r/211062/#review216830

LGTM, thanks!

::: devtools/client/styleeditor/test/browser_styleeditor_fetch-from-netmonitor.js:13
(Diff revision 1)
>  
>  const TEST_URL = TEST_BASE_HTTP + "doc_uncached.html";
>  
>  add_task(function* () {
>    // Disable rcwn to make cache behavior deterministic.
>    yield pushPref("network.http.rcwn.enabled", false);

This could also be removed, it doesn't bring anything anything to this test.

Comment 32

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

Comment on attachment 8940793 [details]
Bug 1306892 - Update style editor fetch test info.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/211062/diff/1-2/

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dfda2fbda09a
Update style editor fetch test info. r=jdescottes

Comment 34

[Bogdan Tara[:bogdan_tara | bogdant]]

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

Comment 35

[Ryan VanderMeulen [:RyanVM]]

We're about a week away from shipping DevEdition from Fx59, so calling this wontfix for Fx58 at this point. Feel free to set the status back to affected and nominate for uplift if you feel strongly otherwise.

Comment 36

[Eliza Balazs [:ebalazs_]]

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