Closed Bug 416896 Opened 17 years ago Closed 17 years ago

[FIX]2.0.0.12 causes <a> elements not to be recognised when inspected in firebug

Categories

(Core :: CSS Parsing and Computation, defect, P3)

1.8 Branch
x86
Windows XP
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: tom.kentell, Assigned: bzbarsky)

References

Details

(Keywords: regression, verified1.8.1.13)

Attachments

(3 files, 1 obsolete file)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.12 When selecting any <a> element with the inspect tool it doesn't display the styles in the style tab. After reverting back to 2.0.0.11 this problem was fixed so I believe it's something that has changed in .12 Reproducible: Always Steps to Reproduce: 1. Open firebug 2. Inspect <a> element 3. Look at style tab Actual Results: No styles displayed, the previous style selected remains in the style tab. Expected Results: Styles applied to the <a> should be displayed.
Component: Extension Compatibility → General
Keywords: qawanted, regression
QA Contact: extension.compatibility → general
Whiteboard: [needs-testcase]
Version: unspecified → 2.0 Branch
There is a firebug error in the console, Error: url has no properties Source File: chrome://firebug/content/lib.js Line: 2041 Here's my guess: one of the calls to Firebug's isSystemURL is getting a null, which causes the error and prevents Firebug from putting up the CSS style info in to the UI. That explains why the users see a failure to inspect styles. Assuming this is what is going on, then the null URL probably comes here: ------------ try { inspectedRules = domUtils ? domUtils.getCSSStyleRules(element) : null; } catch (exc) {} if (inspectedRules) { for (var i = 0; i < inspectedRules.Count(); ++i) { var rule = QI(inspectedRules.GetElementAt(i), nsIDOMCSSStyleRule); var href = rule.parentStyleSheet.href; if (isSystemURL(href)) continue; ------........ Where domUtils is the dom-utils service object.
There is a test case and more info on the Firebug issue: http://code.google.com/p/fbug/issues/detail?id=452
John, just to make sure I understand correctly... You believe this to be a bug in Firebug not in Firefox, right? And I'm presuming this appearing with Firefox 2.0.0.12 is just a coincidence and not a regression on our part.
I now believe there is a problem in Firefox, in domUtils. According to the original report above, there is a change from 11->12. According to Boris, the code in comment #1 should not give null for href. In firebug I get null for rule.parentStyleSheet.href. In the document, the stylesheet href is not null var styleSheetList = document.styleSheets; for (var i = 0; i < styleSheetList.length; i++) document.write(i+")"+styleSheetList[i].href+"<br>"); when put into the test case shows one sheet, the URL of the doc. So: my current guess is that domUtils changed between 11 and 12 wrt to parentStyleSheet.href value. Note that there may be some work on domUtils for FF3 to move it into an extension. John.
Any behavioral change that happens in on branch that causes extensions to fail in some way is normally a bug in Firefox (unless some security thing was fixed and there was no other way to fix it than to break current behavior). Afaik, that's appr. what Boris would say.
Attached file testcase (uses enhanced privileges) (deleted) —
Thanks for that code snippet, that made it relatively easy to make a standalone testcase. This testcase shows the behavioral difference that has occured on branch.
Flags: blocking1.8.1.13?
Keywords: qawanted
Whiteboard: [needs-testcase]
Hmm, with that testcase, using a 2007-12-23 branch build, I get: 0: resource://gre/res/ua.css 1: about:PreferenceStyleSheet 2: about:PreferenceStyleSheet 3: file:///C:/Documents%20and%20Settings/mw/Bureaublad/416896_nullhrefstyle.htm Using a 2007-12-27 branch build, I get: 0: resource://gre/res/ua.css 1: null 2: null 3: file:///C:/Documents%20and%20Settings/mw/Bureaublad/416896_nullhrefstyle.htm
Aha! The testcase helps a ton. This is a regression from bug Bug 397427, indeed. The stylesheet with a null href in this case is the preferences stylesheet. Note that there is no problem with document.styleSheets (as described in comment 4), because the preferences sheet is not in document.styleSheets,
Status: UNCONFIRMED → NEW
Component: General → Style System (CSS)
Ever confirmed: true
Product: Firefox → Core
QA Contact: general → style-system
Version: 2.0 Branch → 1.8 Branch
Assignee: nobody → bzbarsky
Summary: 2.0.0.12 causes <a> elements not to be recognised when inspected in firebug → [FIX]2.0.0.12 causes <a> elements not to be recognised when inspected in firebug
Attached patch Branch fix (obsolete) (deleted) — Splinter Review
This restores the .href of the preferences sheet to being about:whatever.
Attachment #302736 - Flags: superreview?(dbaron)
Attachment #302736 - Flags: review?(dbaron)
Attached patch Trunk version (deleted) — Splinter Review
I think it makes sense to do this for trunk too. It won't affect websites, and will make the firebug output clearer...
Attachment #302737 - Flags: superreview?(dbaron)
Attachment #302737 - Flags: review?(dbaron)
Another question is whether we want to have a way to get at the "real URI" (post-redirect) for chrome code like Firebug.... John, would that be useful?
Flags: blocking1.9?
I don't understand preference sheets, but 'about:whatever' isn't very informative ;-). 'about:PreferenceStyleSheet' is slight informative, but is there a reason the href can't be file: or chrome:? or if there is some security issue, about:PreferenceStyleSheet?url=<encoded_chrome_url>".
Comment on attachment 302736 [details] [diff] [review] Branch fix r+sr=dbaron
Attachment #302736 - Flags: superreview?(dbaron)
Attachment #302736 - Flags: superreview+
Attachment #302736 - Flags: review?(dbaron)
Attachment #302736 - Flags: review+
Comment on attachment 302737 [details] [diff] [review] Trunk version r+sr=dbaron
Attachment #302737 - Flags: superreview?(dbaron)
Attachment #302737 - Flags: superreview+
Attachment #302737 - Flags: review?(dbaron)
Attachment #302737 - Flags: review+
> but 'about:whatever' isn't very informative ;-). I just couldn't recall what exact string came after the ':' when typing that comment. Something to do with preferences. > but is there a reason the href can't be file: or chrome:? There is no file or chrome URI involved, and in fact no real URI at all. The stylesheet is created completely in memory using CSSOM methods plus a non-CSSOM "create a stylesheet object" method. It's like asking why a document created using DOMImplementation.createDocument doesn't have a file or chrome URI...
Comment on attachment 302736 [details] [diff] [review] Branch fix Requesting approval for this branch regression.
Attachment #302736 - Flags: approval1.8.1.13?
Comment on attachment 302737 [details] [diff] [review] Trunk version Simple change that makes Firebug a little more useful. This doesn't affect web content.
Attachment #302737 - Flags: approval1.9?
Comment on attachment 302737 [details] [diff] [review] Trunk version Thnx for the explan bz
Attachment #302737 - Flags: approval1.9? → approval1.9+
There is a way to make firebug works without waiting next update and without using a nightly build?
Are there other places that need the same fix? like http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/mathml/base/src/nsMathMLFrame.cpp&rev=1.53.6.2&mark=555,561#553 (only see that code on the branch)
Flags: blocking1.8.1.13? → blocking1.8.1.13+
Right you are. :( I just double-checked and that's the only other branch caller. There are no other trunk callers.
Attached patch Updated branch patch (deleted) — Splinter Review
Attachment #302736 - Attachment is obsolete: true
Attachment #303112 - Flags: approval1.8.1.13?
Attachment #302736 - Flags: approval1.8.1.13?
Trunk fix checked in, but leaving open since this is originally filed as a branch bug. Someone want to make that testcase into a mochitest?
Flags: in-testsuite?
Attachment #303112 - Flags: superreview+
Attachment #303112 - Flags: review+
Depends on: 417355
I filed bug 417355 for getting the DOM Inspector enabled in the mochitest profile, as it seems to me the mochitest would need to use DOM Inspector code (please correct me if I'm wrong).
dom-utils is part of gklayout, I think.
Ah, sorry, I'm mistaken (in fact, I could have easily checked this myself).
No longer depends on: 417355
For what it's worth, with all the talk about Firebug, this regression is also affecting Chris Pederick's Web Developer Extension. The "View Style Information" on any element work except for on links. Clicking on an anchor will fire the href attribute and follow the link.
Just to make it clear, any extension affected by this is assuming something that contradicts the DOM Style spec, and _will_ break with Gecko 1.9, where inline stylesheets have a null .href, as they should.
Comment on attachment 303112 [details] [diff] [review] Updated branch patch approved for 1.8.1.13, a=dveditz for release-drivers
Attachment #303112 - Flags: approval1.8.1.13? → approval1.8.1.13+
Flags: blocking1.9? → blocking1.9+
Priority: -- → P3
Fixed on branch.
Status: NEW → RESOLVED
Closed: 17 years ago
Keywords: fixed1.8.0.13
Resolution: --- → FIXED
I added a test for this.
Flags: in-testsuite? → in-testsuite+
Verified FIXED using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.13) Gecko/20080311 Firefox/2.0.0.13 http://img221.imageshack.us/my.php?image=picture2gt2.png Replacing fixed1.8.1.13 keyword with verified1.8.1.13
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: