Last Comment Bug 416896 - [FIX]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
Status: RESOLVED FIXED
: regression, verified1.8.1.13
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: 1.8 Branch
: x86 Windows XP
: P3 normal (vote)
: ---
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
:
: Jet Villegas (:jet)
Mentors:
Depends on:
Blocks: CVE-2008-0593
  Show dependency treegraph
 
Reported: 2008-02-11 14:49 PST by Tom Kentell
Modified: 2008-03-17 15:03 PDT (History)
14 users (show)
roc: blocking1.9+
dveditz: blocking1.8.1.13+
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (uses enhanced privileges) (999 bytes, text/html)
2008-02-11 17:31 PST, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
Branch fix (1.44 KB, patch)
2008-02-11 19:01 PST, Boris Zbarsky [:bz] (still a bit busy)
dbaron: review+
dbaron: superreview+
Details | Diff | Splinter Review
Trunk version (1.28 KB, patch)
2008-02-11 19:01 PST, Boris Zbarsky [:bz] (still a bit busy)
dbaron: review+
dbaron: superreview+
mtschrep: approval1.9+
Details | Diff | Splinter Review
Updated branch patch (2.74 KB, patch)
2008-02-13 13:40 PST, Boris Zbarsky [:bz] (still a bit busy)
dbaron: review+
dbaron: superreview+
dveditz: approval1.8.1.13+
Details | Diff | Splinter Review

Description Tom Kentell 2008-02-11 14:49:33 PST
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 2008-02-11 15:34:55 PST
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 2008-02-11 16:59:19 PST
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) 2008-02-11 17:06:36 PST
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 2008-02-11 17:24:41 PST
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 [:mwargers] (not working for Mozilla) 2008-02-11 17:28:34 PST
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 [:mwargers] (not working for Mozilla) 2008-02-11 17:31:09 PST
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.
Comment 7 Martijn Wargers [:mwargers] (not working for Mozilla) 2008-02-11 17:34:45 PST
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 [:bz] (still a bit busy) 2008-02-11 17:41:34 PST
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 [:bz] (still a bit busy) 2008-02-11 19:01:11 PST
Created attachment 302736 [details] [diff] [review]
Branch fix

This restores the .href of the preferences sheet to being about:whatever.
Comment 10 Boris Zbarsky [:bz] (still a bit busy) 2008-02-11 19:01:56 PST
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...
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2008-02-11 19:02:33 PST
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 2008-02-11 22:39:18 PST
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-10 2008-02-11 22:53:11 PST
Comment on attachment 302736 [details] [diff] [review]
Branch fix

r+sr=dbaron
Comment 14 David Baron :dbaron: ⌚️UTC-10 2008-02-11 22:53:54 PST
Comment on attachment 302737 [details] [diff] [review]
Trunk version

r+sr=dbaron
Comment 15 Boris Zbarsky [:bz] (still a bit busy) 2008-02-12 07:51:16 PST
> 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 [:bz] (still a bit busy) 2008-02-12 07:51:24 PST
Comment on attachment 302736 [details] [diff] [review]
Branch fix

Requesting approval for this branch regression.
Comment 17 Boris Zbarsky [:bz] (still a bit busy) 2008-02-12 07:51:51 PST
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 2008-02-12 12:49:12 PST
Comment on attachment 302737 [details] [diff] [review]
Trunk version

Thnx for the explan bz
Comment 19 kNo 2008-02-13 08:38:15 PST
There is a way to make firebug works without waiting next update and without using a nightly build?
Comment 20 Daniel Veditz [:dveditz] 2008-02-13 11:55:49 PST
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 [:bz] (still a bit busy) 2008-02-13 13:35:29 PST
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 [:bz] (still a bit busy) 2008-02-13 13:40:36 PST
Created attachment 303112 [details] [diff] [review]
Updated branch patch
Comment 23 Boris Zbarsky [:bz] (still a bit busy) 2008-02-13 13:49:17 PST
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 [:mwargers] (not working for Mozilla) 2008-02-13 14:47:38 PST
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 [:bz] (still a bit busy) 2008-02-13 15:04:50 PST
dom-utils is part of gklayout, I think.
Comment 26 Martijn Wargers [:mwargers] (not working for Mozilla) 2008-02-13 15:11:32 PST
Ah, sorry, I'm mistaken (in fact, I could have easily checked this myself).
Comment 27 John Lascurettes 2008-02-13 21:57:04 PST
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 [:bz] (still a bit busy) 2008-02-13 23:44:04 PST
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] 2008-02-15 11:17:02 PST
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 [:bz] (still a bit busy) 2008-02-19 09:56:46 PST
Fixed on branch.
Comment 31 Boris Zbarsky [:bz] (still a bit busy) 2008-02-19 10:15:48 PST
I added a test for this.
Comment 32 Stephen Donner [:stephend] 2008-03-17 15:03:56 PDT
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

Note You need to log in before you can comment on or make changes to this bug.