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

Status: RESOLVED FIXED

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

Description

[Tom Kentell]

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.

Comment 1

[John J. Barton]

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

[John J. Barton]

There is a test case and more info on the Firebug issue:
http://code.google.com/p/fbug/issues/detail?id=452

Comment 3

[Samuel Sidler (old account; do not CC)]

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

[John J. Barton]

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

[Martijn Wargers (dead)]

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

[Martijn Wargers (dead)]

Attachment: 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.

Comment 7

[Martijn Wargers (dead)]

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

Comment 8

[Boris Zbarsky [:bzbarsky]]

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,

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Branch fix

This restores the .href of the preferences sheet to being about:whatever.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: 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...

Comment 11

[Boris Zbarsky [:bzbarsky]]

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?

Comment 12

[John J. Barton]

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 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 302736 [details] [diff] [review]
Branch fix

r+sr=dbaron

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 302737 [details] [diff] [review]
Trunk version

r+sr=dbaron

Comment 15

[Boris Zbarsky [:bzbarsky]]

> 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 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 302736 [details] [diff] [review]
Branch fix

Requesting approval for this branch regression.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 302737 [details] [diff] [review]
Trunk version

Simple change that makes Firebug a little more useful.  This doesn't affect web content.

Comment 18

[Mike Schroepfer]

Comment on attachment 302737 [details] [diff] [review]
Trunk version

Thnx for the explan bz

Comment 19

[kNo]

There is a way to make firebug works without waiting next update and without using a nightly build?

Comment 20

[Daniel Veditz [:dveditz]]

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)

Comment 21

[Boris Zbarsky [:bzbarsky]]

Right you are.  :(

I just double-checked and that's the only other branch caller.  There are no other trunk callers.

Comment 22

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated branch patch

Comment 23

[Boris Zbarsky [:bzbarsky]]

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?

Comment 24

[Martijn Wargers (dead)]

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).

Comment 25

[Boris Zbarsky [:bzbarsky]]

dom-utils is part of gklayout, I think.

Comment 26

[Martijn Wargers (dead)]

Ah, sorry, I'm mistaken (in fact, I could have easily checked this myself).

Comment 27

[John Lascurettes]

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.

Comment 28

[Boris Zbarsky [:bzbarsky]]

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

[Daniel Veditz [:dveditz]]

Comment on attachment 303112 [details] [diff] [review]
Updated branch patch

approved for 1.8.1.13, a=dveditz for release-drivers

Comment 30

[Boris Zbarsky [:bzbarsky]]

Fixed on branch.

Comment 31

[Boris Zbarsky [:bzbarsky]]

I added a test for this.

Comment 32

[Stephen Donner [:stephend] Not actively reading bugmail]

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