Last Comment Bug 414856 - Firefox 2.0.0.12 RC1 breaks Stylish with "TypeError: stylesheet has no properties"
: Firefox 2.0.0.12 RC1 breaks Stylish with "TypeError: stylesheet has no proper...
Status: VERIFIED FIXED
: regression, testcase, verified1.8.1.12, verified1.8.1.13
Product: Core
Classification: Components
Component: XML (show other bugs)
: 1.8 Branch
: x86 Windows XP
: -- normal (vote)
: ---
Assigned To: Olli Pettay [:smaug] (vacation Aug 25-28)
:
Mentors:
Depends on:
Blocks: 382636 414327
  Show dependency treegraph
 
Reported: 2008-01-30 07:59 PST by Jason Barnabe (np)
Modified: 2008-03-17 16:28 PDT (History)
15 users (show)
dveditz: blocking1.8.1.12+
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (795 bytes, text/html)
2008-01-30 11:27 PST, Olli Pettay [:smaug] (vacation Aug 25-28)
no flags Details
revert back the old behavior (1.00 KB, patch)
2008-01-30 11:35 PST, Olli Pettay [:smaug] (vacation Aug 25-28)
jonas: review+
jonas: superreview+
dveditz: approval1.8.1.12+
Details | Diff | Splinter Review
testcase, works on Opera and Safari too (737 bytes, text/html)
2008-01-30 11:52 PST, Olli Pettay [:smaug] (vacation Aug 25-28)
no flags Details

Description Jason Barnabe (np) 2008-01-30 07:59:55 PST
With Stylish 0.5.3 and Firefox 2.0.0.12 RC1:

1. Click on the Stylish icon, Write Style, For this URL...
2. Preview

Result: TypeError: stylesheet has no properties

This worked with 2.0.0.11.

I'm the author of Stylish and I can make changes to it if required.
Comment 1 Jason Barnabe (np) 2008-01-30 08:12:32 PST
Stylish is trying to get a nsIDOMDocumentStyle from a string of CSS text. To do this, it:

1. Creates a document: document.implementation.createDocument(stylishCommon.XULNS, "stylish-parse", null)
2. Appends a link element with a data URI to the document's document element.
3. Gets a stylesheet object out of the document: doc.QueryInterface(Components.interfaces.nsIDOMDocumentStyle).styleSheets[0]
4. Checks to make sure it's done loading. It's expecting stylesheet.cssRules.length to throw NS_ERROR_DOM_INVALID_ACCESS_ERR if it's not done. Instead, 'stylesheet' is undefined.
Comment 2 Jason Barnabe (np) 2008-01-30 08:16:50 PST
'stylesheet' never gets defined, even on subsequent checks.
Comment 3 Mats Palmgren (:mats) 2008-01-30 09:46:43 PST
Olli, could this be a regression from bug 393762?
Comment 5 Jason Barnabe (np) 2008-01-30 10:00:56 PST
2008-01-22-04-mozilla1.8 - Bug not present
2008-01-23-03-mozilla1.8 - Bug present
Comment 6 Olli Pettay [:smaug] (vacation Aug 25-28) 2008-01-30 10:44:51 PST
Any chance to get a minimal testcase, attached using "Add an attachment"
Comment 7 Olli Pettay [:smaug] (vacation Aug 25-28) 2008-01-30 10:57:54 PST
Haven't yet tested, but my guess is that since loading external files from
data document (which are now created when .createDocument is used) isn't
allowed anymore, loading stylesheets using a data document doesn't work either.
Comment 8 Olli Pettay [:smaug] (vacation Aug 25-28) 2008-01-30 11:00:59 PST
So this is a regression from bug 382636.
Do we want to allow loading external files on branch, but perhaps not on trunk?
... I need to still verify that that is the case here.
Comment 9 Olli Pettay [:smaug] (vacation Aug 25-28) 2008-01-30 11:27:13 PST
Created attachment 300429 [details]
testcase
Comment 10 Olli Pettay [:smaug] (vacation Aug 25-28) 2008-01-30 11:35:00 PST
Created attachment 300432 [details] [diff] [review]
revert back the old behavior

Do we want to use this? Jonas, bz, anyone?
Doesn't regress Bug 393762 or Bug 393761
Comment 11 Olli Pettay [:smaug] (vacation Aug 25-28) 2008-01-30 11:52:50 PST
Created attachment 300438 [details]
testcase, works on Opera and Safari too

Opera gives the same result as 2.0.11, Safari same as trunk
Comment 12 Boris Zbarsky [:bz] 2008-01-30 11:57:58 PST
I thought we already set createDocument() documents to be data documents...  If we don't, that seems like a bug.

I'm not sure about the security aspect of it, though.  If it's safe, it might indeed make sense to revert that part for compat reasons.
Comment 13 Olli Pettay [:smaug] (vacation Aug 25-28) 2008-01-30 12:01:41 PST
No we didn't set .createDocuments as data documents.
It is IMO a bug, which is now fixed both on branch (sort of accidentally) and on
trunk (on purpose).
The extension must be changed for FF3, but during FF2 it is better to not change
the behavior unless absolutely needed.
Comment 14 Daniel Veditz [:dveditz] 2008-01-30 12:28:45 PST
It's not just the extension, the testcase shows this change affects web content. Dunno if anyone uses it, but it's definitely a potentially web-breaking change not just an addon problem.
Comment 15 Daniel Veditz [:dveditz] 2008-01-30 12:34:10 PST
Comment on attachment 300432 [details] [diff] [review]
revert back the old behavior

approved for 1.8.1.12, a=dveditz for release-drivers
Comment 16 Olli Pettay [:smaug] (vacation Aug 25-28) 2008-01-30 12:37:43 PST
Jason, could you please try next nightlies. Thanks!
Comment 17 Olli Pettay [:smaug] (vacation Aug 25-28) 2008-01-30 12:50:58 PST
Checked in to GECKO181_20080128_RELBRANCH and MOZILLA_1_8_BRANCH 
Comment 18 Boris Zbarsky [:bz] 2008-01-30 13:00:32 PST
Please get some mochitests in?
Comment 19 Olli Pettay [:smaug] (vacation Aug 25-28) 2008-01-30 13:04:36 PST
To test the current trunk behavior? Or branch behavior?
Comment 20 Olli Pettay [:smaug] (vacation Aug 25-28) 2008-01-30 13:05:33 PST
I guess trunk. That is IMO what we want, or otherwise we should allow
also image loads etc on data documents.
Comment 21 Daniel Veditz [:dveditz] 2008-01-30 14:26:43 PST
The primary bugs to regression test are of course bug 393762 and bug 393761.

This will, of course, regress bug 382636 since this bug is basically about backing that one out. That is expected and OK for the 1.8 branch.

Make sure it doesn't regress old bug 325005 (which checks the "loadedAsData"
flag).
Comment 22 Al Billings [:abillings] 2008-01-30 17:46:07 PST
This is fixed in Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.12) Gecko/2008013015 Firefox/2.0.0.1. I've verified it.
Comment 24 Stephen Donner [:stephend] 2008-03-17 14:31:16 PDT
(In reply to comment #23)
> Verified FIXED using Bonsai:
> 
> http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=shipped-locales&branch=MOZILLA_1_8_BRANCH&root=/cvsroot&subdir=/mozilla/browser/locales&command=DIFF_FRAMESET&rev1=1.1.4.18&rev2=1.1.4.19
> 
> Replacing fixed1.8.1.13 keyword with verified1.8.1.13.

Commented in the wrong bug, sorry, restoring fixed1.8.1.13 keyword.
Comment 25 Al Billings [:abillings] 2008-03-17 16:28:12 PDT
This is verified for 1.8.1.13 as well.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.13) Gecko/2008031114 Firefox/2.0.0.13

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