Bug 114761

Summary: Computed style of fill/stroke properties incorrect on references
Product: WebKit Reporter: Dirk Schulze <krit>
Component: SVGAssignee: Rob Buis <rwlbuis>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, d-r, esprehn+autocc, fmalita, glenn, macpherson, menard, pdr, rniwa, rwlbuis, schenney, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from webkit-cq-02 for mac-mountainlion
none
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion
none
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2
none
Patch none

Description Dirk Schulze 2013-04-17 13:28:42 PDT
Computed style of fill/stroke properties incorrect on references. The output just has the reference without the url() function.
Comment 1 Dirk Schulze 2013-04-17 13:34:45 PDT
Created attachment 198604 [details]
Patch
Comment 2 Andreas Kling 2013-04-17 13:37:59 PDT
Comment on attachment 198604 [details]
Patch

r=me
Comment 3 WebKit Commit Bot 2013-04-17 14:20:22 PDT
Comment on attachment 198604 [details]
Patch

Rejecting attachment 198604 [details] from commit-queue.

New failing tests:
transitions/svg-transitions.html
Full output: http://webkit-queues.appspot.com/results/102492
Comment 4 WebKit Commit Bot 2013-04-17 14:20:23 PDT
Created attachment 198610 [details]
Archive of layout-test-results from webkit-cq-02 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: webkit-cq-02  Port: mac-mountainlion  Platform: Mac OS X 10.8.3
Comment 5 Build Bot 2013-04-17 18:29:09 PDT
Comment on attachment 198604 [details]
Patch

Attachment 198604 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/65144

New failing tests:
transitions/svg-transitions.html
Comment 6 Build Bot 2013-04-17 18:29:10 PDT
Created attachment 198667 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.2
Comment 7 Build Bot 2013-04-17 23:49:53 PDT
Comment on attachment 198604 [details]
Patch

Attachment 198604 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/21329

New failing tests:
transitions/svg-transitions.html
Comment 8 Build Bot 2013-04-17 23:49:55 PDT
Created attachment 198683 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.2
Comment 9 Dirk Schulze 2013-04-18 10:07:55 PDT
(In reply to comment #8)
> Created an attachment (id=198683) [details]
> Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2
> 
> The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
> Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.2

Not sure if someone updates a result in the meantime, but I do not get this failure. Can some one post the output if it still occurs please?
Comment 10 Dirk Schulze 2013-04-18 10:09:10 PDT
Never mind, see the output from the bot :P
Comment 11 Rob Buis 2013-08-25 16:04:25 PDT
Created attachment 209607 [details]
Patch
Comment 12 Darin Adler 2013-08-26 10:25:38 PDT
Comment on attachment 209607 [details]
Patch

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

> Source/WebCore/css/CSSPrimitiveValue.cpp:1003
> +            text = "url(" + quoteCSSURLIfNeeded(m_value.string) + ')';

Just snuck in a related optimization?
Comment 13 Rob Buis 2013-08-26 11:37:59 PDT
(In reply to comment #12)
> (From update of attachment 209607 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=209607&action=review
> 
> > Source/WebCore/css/CSSPrimitiveValue.cpp:1003
> > +            text = "url(" + quoteCSSURLIfNeeded(m_value.string) + ')';
> 
> Just snuck in a related optimization?

Yes :) I figured since it was part of Dirk's original patch and Andreas okayed it it may as well go in like that.
Comment 14 WebKit Commit Bot 2013-08-26 12:02:43 PDT
Comment on attachment 209607 [details]
Patch

Clearing flags on attachment: 209607

Committed r154628: <http://trac.webkit.org/changeset/154628>
Comment 15 WebKit Commit Bot 2013-08-26 12:02:48 PDT
All reviewed patches have been landed.  Closing bug.
Comment 16 Dirk Schulze 2013-08-27 02:45:04 PDT
Did I forget to land it?
Comment 17 Rob Buis 2013-08-27 06:56:53 PDT
(In reply to comment #16)
> Did I forget to land it?

The patch never landed AFAICT.