The default bug view has changed. See this FAQ.

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

RESOLVED FIXED

Status

()

Core
CSS Parsing and Computation
P3
normal
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Tom Kentell, Assigned: bz)

Tracking

({regression, verified1.8.1.13})

1.8 Branch
x86
Windows XP
regression, verified1.8.1.13
Points:
---
Bug Flags:
blocking1.9 +
blocking1.8.1.13 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

9 years ago
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

Comment 1

9 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

9 years ago
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.

Comment 4

9 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.
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.
Created attachment 302721 [details]
testcase (uses enhanced privileges)

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
Created attachment 302736 [details] [diff] [review]
Branch fix

This restores the .href of the preferences sheet to being about:whatever.
Attachment #302736 - Flags: superreview?(dbaron)
Attachment #302736 - Flags: review?(dbaron)
Created attachment 302737 [details] [diff] [review]
Trunk version

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?

Comment 12

9 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>".  
Blocks: 397427
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 18

9 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

9 years ago
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.
Created attachment 303112 [details] [diff] [review]
Updated branch patch
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+

Updated

9 years ago
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

Comment 27

9 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.
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
Last Resolved: 9 years ago
Keywords: fixed1.8.0.13
Resolution: --- → FIXED
I added a test for this.
Flags: in-testsuite? → in-testsuite+
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.