Bug 81859

Summary: Chromium: Should enable -webkit-image-set
Product: WebKit Reporter: Beth Dakin <bdakin>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, eoconnor, flackr, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 83921    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ec2-cr-linux-02
none
Archive of layout-test-results from ec2-cr-linux-01
none
Patch
none
Archive of layout-test-results from ec2-cr-linux-02
none
Patch none

Description Beth Dakin 2012-03-21 18:18:34 PDT
https://bugs.webkit.org/show_bug.cgi?id=80322 adds a new CSS property called -webkit-image-set. Since the implementation is currently incomplete, ports have to opt-in. Chrome should decide when to opt in.
Comment 1 Robert Flack 2012-04-11 11:11:54 PDT
Created attachment 136710 [details]
Patch
Comment 2 WebKit Review Bot 2012-04-11 11:15:04 PDT
Attachment 136710 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1
Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Robert Kroeger 2012-04-11 12:06:51 PDT
Comment on attachment 136710 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=136710&action=review

change looks good. add the image expectations

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

you must change this line. it will not pass the commit queue.
Comment 4 WebKit Review Bot 2012-04-11 13:42:34 PDT
Comment on attachment 136710 [details]
Patch

Attachment 136710 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12387257

New failing tests:
fast/css/image-set-parsing.html
Comment 5 WebKit Review Bot 2012-04-11 13:42:39 PDT
Created attachment 136743 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 6 WebKit Review Bot 2012-04-11 14:57:10 PDT
Comment on attachment 136710 [details]
Patch

Attachment 136710 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12392020

New failing tests:
fast/css/image-set-parsing.html
Comment 7 WebKit Review Bot 2012-04-11 14:57:16 PDT
Created attachment 136759 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 8 Robert Flack 2012-04-12 08:50:56 PDT
Created attachment 136919 [details]
Patch
Comment 9 WebKit Review Bot 2012-04-12 10:52:48 PDT
Comment on attachment 136919 [details]
Patch

Attachment 136919 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12392491

New failing tests:
fast/css/image-set-parsing.html
Comment 10 WebKit Review Bot 2012-04-12 10:52:54 PDT
Created attachment 136933 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 11 Robert Flack 2012-04-12 14:58:05 PDT
Created attachment 136979 [details]
Patch
Comment 12 Adam Barth 2012-04-12 15:54:45 PDT
Comment on attachment 136979 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=136979&action=review

> LayoutTests/platform/chromium/fast/css/image-set-parsing-expected.txt:58
> +FAIL jsWrapperClass(imageSetRule.__proto__) should be CSSValueListPrototype. Was Object.
> +FAIL jsWrapperClass(imageSetRule.constructor) should be CSSValueListConstructor. Was Function.

Why do we fail these subtests?
Comment 13 Robert Flack 2012-04-12 17:19:53 PDT
(In reply to comment #12)
> (From update of attachment 136979 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=136979&action=review
> 
> > LayoutTests/platform/chromium/fast/css/image-set-parsing-expected.txt:58
> > +FAIL jsWrapperClass(imageSetRule.__proto__) should be CSSValueListPrototype. Was Object.
> > +FAIL jsWrapperClass(imageSetRule.constructor) should be CSSValueListConstructor. Was Function.
> 
> Why do we fail these subtests?

It seems we do this for all the chromium tests which use toString to verify the class prototype and constructor since chromium always returns Object and Function for these respectively. For example for the prototype:  
egrep --exclude-dir=".svn" -r "jsWrapperClass\(.*.__proto__\) should be [A-Za-z]*\. Was Object\." . |wc -l
155
Comment 14 Adam Barth 2012-04-12 17:26:32 PDT
Comment on attachment 136979 [details]
Patch

Interesting.  I wonder when that started.
Comment 15 WebKit Review Bot 2012-04-13 10:20:42 PDT
Comment on attachment 136979 [details]
Patch

Clearing flags on attachment: 136979

Committed r114140: <http://trac.webkit.org/changeset/114140>
Comment 16 WebKit Review Bot 2012-04-13 10:21:11 PDT
All reviewed patches have been landed.  Closing bug.
Comment 18 Robert Flack 2012-04-13 14:30:43 PDT
Relanded in http://trac.webkit.org/changeset/114163. The features.gypi change required clobbering the windows chromium bots. I don't think I have permission to close the issue.
Comment 19 Adam Barth 2012-04-13 14:33:08 PDT
Robert, if you ask eseidel in #webkit, he should be able to give you the EditBugs permission.