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)
Tracking
()
RESOLVED
FIXED
People
(Reporter: tom.kentell, Assigned: bzbarsky)
References
Details
(Keywords: regression, verified1.8.1.13)
Attachments
(3 files, 1 obsolete file)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
dbaron
:
review+
dbaron
:
superreview+
mtschrep
:
approval1.9+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dbaron
:
review+
dbaron
:
superreview+
dveditz
:
approval1.8.1.13+
|
Details | Diff | Splinter Review |
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.
Updated•17 years ago
|
Component: Extension Compatibility → General
Keywords: qawanted,
regression
QA Contact: extension.compatibility → general
Whiteboard: [needs-testcase]
Version: unspecified → 2.0 Branch
Comment 1•17 years ago
|
||
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.
Comment 2•17 years ago
|
||
There is a test case and more info on the Firebug issue:
http://code.google.com/p/fbug/issues/detail?id=452
Comment 3•17 years ago
|
||
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.
Comment 4•17 years ago
|
||
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.
Comment 5•17 years ago
|
||
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.
Comment 6•17 years ago
|
||
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.
Updated•17 years ago
|
Flags: blocking1.8.1.13?
Comment 7•17 years ago
|
||
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
Assignee | ||
Comment 8•17 years ago
|
||
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 | ||
Updated•17 years ago
|
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
Assignee | ||
Comment 9•17 years ago
|
||
This restores the .href of the preferences sheet to being about:whatever.
Attachment #302736 -
Flags: superreview?(dbaron)
Attachment #302736 -
Flags: review?(dbaron)
Assignee | ||
Comment 10•17 years ago
|
||
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)
Assignee | ||
Comment 11•17 years ago
|
||
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?
Updated•17 years ago
|
Flags: blocking1.9?
Comment 12•17 years ago
|
||
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>".
Updated•17 years ago
|
Blocks: CVE-2008-0593
Comment 13•17 years ago
|
||
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 14•17 years ago
|
||
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+
Assignee | ||
Comment 15•17 years ago
|
||
> 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...
Assignee | ||
Comment 16•17 years ago
|
||
Comment on attachment 302736 [details] [diff] [review]
Branch fix
Requesting approval for this branch regression.
Attachment #302736 -
Flags: approval1.8.1.13?
Assignee | ||
Comment 17•17 years ago
|
||
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 18•17 years ago
|
||
Comment on attachment 302737 [details] [diff] [review]
Trunk version
Thnx for the explan bz
Attachment #302737 -
Flags: approval1.9? → approval1.9+
Comment 19•17 years ago
|
||
There is a way to make firebug works without waiting next update and without using a nightly build?
Comment 20•17 years ago
|
||
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+
Assignee | ||
Comment 21•17 years ago
|
||
Right you are. :(
I just double-checked and that's the only other branch caller. There are no other trunk callers.
Assignee | ||
Comment 22•17 years ago
|
||
Attachment #302736 -
Attachment is obsolete: true
Attachment #303112 -
Flags: approval1.8.1.13?
Attachment #302736 -
Flags: approval1.8.1.13?
Assignee | ||
Comment 23•17 years ago
|
||
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?
Updated•17 years ago
|
Attachment #303112 -
Flags: superreview+
Attachment #303112 -
Flags: review+
Comment 24•17 years ago
|
||
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).
Assignee | ||
Comment 25•17 years ago
|
||
dom-utils is part of gklayout, I think.
Comment 26•17 years ago
|
||
Ah, sorry, I'm mistaken (in fact, I could have easily checked this myself).
No longer depends on: 417355
Comment 27•17 years ago
|
||
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.
Assignee | ||
Comment 28•17 years ago
|
||
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 29•17 years ago
|
||
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
Assignee | ||
Comment 30•17 years ago
|
||
Fixed on branch.
Updated•17 years ago
|
Keywords: fixed1.8.0.13 → fixed1.8.1.13
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
Keywords: fixed1.8.1.13 → verified1.8.1.13
You need to log in
before you can comment on or make changes to this bug.
Description
•