Closed Bug 1249888 Opened 9 years ago Closed 9 years ago

Inspector doesn't show style rules in developer edition

Categories

(DevTools :: Inspector: Rules, defect, P1)

Product:
DevTools
Component:
Inspector: Rules
Version:
46 Branch
Type:
defect

Tracking

(firefox45 unaffected, firefox46+ fixed, firefox47+ fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 47
Tracking Flags:
Tracking Status
firefox45 --- unaffected
firefox46 + fixed
firefox47 + fixed

People

(Reporter: cvetan.simsic, Assigned: pbro)

References

Details

(Keywords: regression, Whiteboard: [btpp-fix-now])

Attachments

(3 files)

firefox-inspector.png
(deleted), image/png
Details
Rules are not shown on FF46Dev
(deleted), image/png
Details
MozReview Request: Bug 1249888 - try/catch SourceMapConsumer to avoid empty rule-view when source map is invalid; r=gl
(deleted), text/x-review-board-request
gl
: review+
lizzard
: approval-mozilla-aurora+
Details
Attached image firefox-inspector.png (deleted) —
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:44.0) Gecko/20100101 Firefox/44.0 Build ID: 20160209234642 Steps to reproduce: Inspect some element like text field, text area etc... Inspector should show style rules but it doesn't.
Component: Untriaged → Developer Tools: Inspector
Could you attach a testcase to reproduce the issue when opening Inspector, please.
Component: Developer Tools: Inspector → Developer Tools: CSS Rules Inspector
Flags: needinfo?(cvetan.simsic)
I've put attachment in description. There is screenshot of inspector window. If something else is needed, please instruct me.
Flags: needinfo?(cvetan.simsic)
It doesn't really help, because the code is partially interpreted. In addition, we need the CSS sheet with the HTML source.
It's a bootstrap min css, and some of my cystom css. But in regular firefox it shows all normal. It looks like it doesn't display rules from minimized css files although i have some custom css also in the element i am trying to inspect.
Hi want to second this problem on FF Ubuntu 46.0a2 (2016-02-21). Rules are not shown when bootstrap is attached to the webpage.
Attached image Rules are not shown on FF46Dev (deleted) —
I could reproduce by visiting http://uni-wuerzburg.de/scripts/ (seen in the attached png) and then opening the inspector. I see this in the browser console: GET http://uni-wuerzburg.de/scripts/static/bootstrap.css.map [HTTP/1.0 404 Not Found 206ms] ... which is suggestive. If I then open the <header> in the inspector and click the <nav>, I get an error: A promise chain failed to handle a rejection. Did you forget to '.catch', or did you forget to 'return'? See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise Date: Mon Feb 22 2016 09:03:28 GMT-0700 (MST) Full Message: SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data Full Stack: SourceMapConsumer@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/sourcemap/source-map.js:1281:20 StyleSheetActor<._fetchSourceMap/</map<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/stylesheets.js:753:21 Handler.prototype.process@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:937:23 this.PromiseWalker.walkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:816:7 Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:747:11 this.PromiseWalker.schedulePromise@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:779:7 this.PromiseWalker.completePromise@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/Promise-backend.js:714:7 mainThreadFetch/onResponse@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/DevToolsUtils.js:442:7 NetUtil_asyncFetch/<.onStopRequest@resource://gre/modules/NetUtil.jsm:128:17 source-map.js:1281:0 A promise chain failed to handle a rejection. Did you forget to '.catch', or did you forget to 'return'? See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
OS: Unspecified → Linux
Hardware: Unspecified → x86_64
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5f9ba76eb3b1fd9377bbdb4cc2f98a7e75eabdfb&tochange=ce6b2fdabc3ecd709abf5de6dbe963edd44d18fb Bug 1243749 - Enable 2 rule-view test with e10s and fix unhandled rejected promises; r=miker Bug 1243736 - Enable browser_rules_pseudo-element_02.js with e10s and make rule-view wait for updateSourceLink; r=bgrins
status-firefox45: --- → unaffected
status-firefox46: --- → affected
status-firefox47: --- → affected
tracking-firefox46: --- → ?
tracking-firefox47: --- → ?
Flags: needinfo?(pbrosset)
Keywords: regression
OS: Linux → All
Hardware: x86_64 → All
I was able to see the errors mentioned in comment 7 with FF46 but couldn't get an empty rules-view. However with FF47, I see both the errors and the rules-view is empty. It indeed looks like a problem with a source map not being found. And because the server returns a 404 HTML page: "<html><head><title>404 Not found</title><meta http-equiv="refresh" CONTENT="0;URL=http://uni-wuerzburg.de/not_found/"></head><body><h1>HTTP/1.0 404 Not Found</h1><!-- Redirect URL is http://uni-wuerzburg.de/not_found/ --></body></html>" then the source-map code fails when calling JSON.parse on that string, which it does here: https://dxr.mozilla.org/mozilla-central/source/devtools/shared/sourcemap/source-map.js#1281 Which, in fact, comes from an upstream repo on github: https://github.com/mozilla/source-map/blob/master/lib/source-map-consumer.js The code responsible for using the source-map-consumer is _fetchSourceMap in the StyleSheetActor in \devtools\server\actors\stylesheets.js It seems to me like we should: - try/catch JSON.parse calls in the source-map library - and listen for 404 or errors when fetching the source map and bailing out
Flags: needinfo?(pbrosset)
Filed this issue: https://github.com/mozilla/source-map/issues/225 But we still need to fix the issue in stylesheets.js. This can be done pretty easily in _fetchSourceMap, the promise returned by fetch actually also provides the contentType of the response. So we could do something like: // Fetching the source map might have failed with a 404 or other. Make // sure the source map is somethig that SourceMapConsumer can parse. if (contentType !== "text/plain") { deferred.reject(new Error("Source map was not found")); return; }
Blocks: 1243749
Seems like a pretty serious regression we should fix soon.
Priority: P2 → P1
Whiteboard: [btpp-fix-now]
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Attachment #8723478 - Flags: review?(gl)
Comment on attachment 8723478 [details] MozReview Request: Bug 1249888 - try/catch SourceMapConsumer to avoid empty rule-view when source map is invalid; r=gl https://reviewboard.mozilla.org/r/36579/#review33491 ::: devtools/client/inspector/rules/test/browser_rules_invalid-source-map.js:15 (Diff revision 1) > + info("Setting the " + PREF + " pref to true"); I personally prefer to not repeat what can be inferred from the function. I think we should remove: L15 - info("Setting the " + PREF + " pref to true"); L21 - info("Selecting the test node"); since we can infer what is being done from reading the code. ::: devtools/client/inspector/rules/test/browser_rules_invalid-source-map.js:39 (Diff revision 1) > +function verifyLinkText(text, view) { I prefer to see view as the first argument. That is consistent in some functions we have in the rules view.
Attachment #8723478 - Flags: review+
Comment on attachment 8723478 [details] MozReview Request: Bug 1249888 - try/catch SourceMapConsumer to avoid empty rule-view when source map is invalid; r=gl https://reviewboard.mozilla.org/r/36579/#review33497
(In reply to Gabriel Luong [:gl] (ΦωΦ) from comment #15) > I personally prefer to not repeat what can be inferred from the function. > > I think we should remove: > L15 - info("Setting the " + PREF + " pref to true"); > L21 - info("Selecting the test node"); > since we can infer what is being done from reading the code. I completely agree with you for production code: way better to make the code readable and avoid the extra comment. Comments are hard to keep in sync and tend to become obsolete. I only agree with you for test code to some extent though: It's also very important to make the code readable, it's code that will have to be maintained too, but the big difference is that with tests, we spend hours reading through logs either locally or on treeherder. So you may have the perfect code, very easily readable, but if you don't have any info() statement in it, then the logs are useless when the test starts failing. By all means, if the code is easy to read, no need for line/block comments (// or /**/), but a healthy does of info("doing this now") sprinkled at the right places in the test is a huge help! Having said this, the 2 places you mention don't require info() statements, so I will remove them. The pref is set before adding the tab, and when adding a tab, there's an info() added already, so we're fine. And selectNode already has an info() too, so no need for another one here.
https://hg.mozilla.org/mozilla-central/rev/059d7b0fa764
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox47: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Comment on attachment 8723478 [details] MozReview Request: Bug 1249888 - try/catch SourceMapConsumer to avoid empty rule-view when source map is invalid; r=gl Approval Request Comment [Feature/regressing bug #]: This regression was introduced by the fix in bug 1243736. That fix made the Rules panel wait for the resolution of source maps. Which means that if a source map is missing, the panel will never load. [User impact if declined]: Pages that load a CSS file referencing an invalid or inexistant source map will cause the Rules panel in the inspector to be empty. [Describe test coverage new/current, TreeHerder]: A new devtools mochitest was added for this fix. The patch has landed on m-c. [Risks and why]: The fix itself is a simple try/catch block around the code that consumes the source map, to fail silently if it isn't found or invalid. So this is low risk. [String/UUID change made/needed]: None
Attachment #8723478 - Flags: approval-mozilla-aurora?
Blocks: 1243736
No longer blocks: 1243749
Tracking since this is a regression from 46.
tracking-firefox46: ? → +
tracking-firefox47: ? → +
Comment on attachment 8723478 [details] MozReview Request: Bug 1249888 - try/catch SourceMapConsumer to avoid empty rule-view when source map is invalid; r=gl Minor regression from 46, needed for dev tools inspector to work correctly with e10s.
Attachment #8723478 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Still fails on 47.0a2 with Error: Connection closed, pending request to server1.conn5.child1/53, type getOriginalLocation failed
Should I need to provide a testcase or sth in order to reopen this bug?, I've tried Aurora & Nightly and I can still reproduce it
(In reply to Hiram J. Perez from comment #28) > Should I need to provide a testcase or sth in order to reopen this bug?, > I've tried Aurora & Nightly and I can still reproduce it Open a new bug report and attach your testcase, then copy here the bug link as follow-up.
(In reply to Hiram J. Perez from comment #30) > Follow-up: https://bugzilla.mozilla.org/show_bug.cgi?id=1260510 Thanks for filing a new bug, this is a different issue. I will comment in this other bug.
[bugday-20160323] Status: RESOLVED,FIXED -> VERIFIED Comments: Test Successful Component: Name Firefox Version 46.0b9 Build ID 20160322075646 Update Channel beta User Agent Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0 OS Windows 7 SP1 x86_64 Expected Results: Yes Actual Results: As expected
I have reproduced this bug with Firefox aurora 46.0a2(build ID:20160220004011)on windows 7(64 bit) I have verified this bug as fixed with Firefox nightly 48.0a1(build ID:20160421030302) User Agent:Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: