Bug 31224

Summary: [V8] Return StyleSheet object instead of HTMLStyleElement w/document.styleSheets named property getter.
Product: WebKit Reporter: johnnyding <johnnyding.webkit>
Component: WebCore JavaScriptAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dglazkov
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
URL: http://soren.qzone.qq.com/cgi-bin/soren/cgi_userinfo_search?bSimple=0&ptlang=2052
Attachments:
Description Flags
patch to fix this issue.
dglazkov: review-
patch v2 for fix
none
patch v3 for fix
none
patch v3 for fix none

Description johnnyding 2009-11-07 08:01:05 PST
In page http://soren.qzone.qq.com/cgi-bin/soren/cgi_userinfo_search?bSimple=0&ptlang=2052, it uses document.styleSheets["CssId"].rules to get style css rule list.

In safari, the page run well, but in Google Chrome, accessing document.styleSheets["CssId"].rules got "undefined".

After investigation, Safari returns a [CSSStyleSheet] object when accessing document.styleSheets["CssId"], so the "rules" can be gotten. Chrome returns a [HTMLStyleElement] object when accessing document.styleSheets["CssId"], so the "rules" can not be gotten.

I believe it's v8 binding's fault. See JS binding (JSStyleSheetListCustom.cpp, line 60), it returns stylesheet instead of returning style element.
Comment 1 johnnyding 2009-11-07 08:41:18 PST
Created attachment 42694 [details]
patch to fix this issue.
Comment 2 Dimitri Glazkov (Google) 2009-11-07 08:45:05 PST
Comment on attachment 42694 [details]
patch to fix this issue.

Thanks for tackling this! We just need a layout test and one small nit:

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 50615)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,16 @@
> +2009-11-07  Johnny Ding  <johnnyding.webkit@gmail.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        https://bugs.webkit.org/show_bug.cgi?id=31224
> +        Returns a StyleSheet object instead of HTMLStyleElement when using

Please update this to match the subject of the bug (including [V8] prefix).

> +        V8 engine to retrieve style sheet by a name from document.styleSheets


.
> +
> +        No new tests since it is only V8 binding issue.(JS binding is OK)

Well, this is an easily testable case, so I would say a layout test is in order, regardless
of whether it's a bindings issue for one engine and not another.
Comment 3 Dimitri Glazkov (Google) 2009-11-07 10:01:04 PST
I wonder if there's already a layout test that we're failing that tests this.
Comment 4 johnnyding 2009-11-07 10:09:08 PST
Created attachment 42696 [details]
patch v2 for fix

Thanks, patch v2 is coming:)
Comment 5 johnnyding 2009-11-07 11:16:01 PST
Created attachment 42698 [details]
patch v3 for fix
Comment 6 johnnyding 2009-11-07 11:18:09 PST
Created attachment 42699 [details]
patch v3 for fix
Comment 7 Dimitri Glazkov (Google) 2009-11-08 21:38:30 PST
Comment on attachment 42699 [details]
patch v3 for fix

Lovely. r=me.
Comment 8 WebKit Commit Bot 2009-11-08 21:52:47 PST
Comment on attachment 42699 [details]
patch v3 for fix

Clearing flags on attachment: 42699

Committed r50637: <http://trac.webkit.org/changeset/50637>
Comment 9 WebKit Commit Bot 2009-11-08 21:52:52 PST
All reviewed patches have been landed.  Closing bug.