Bug 131094 - Remove one of the CSSProperty constructor
Summary: Remove one of the CSSProperty constructor
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-02 01:21 PDT by Zsolt Borbely
Modified: 2014-05-24 20:13 PDT (History)
8 users (show)

See Also:


Attachments
Proposed patch (2.06 KB, patch)
2014-04-02 01:24 PDT, Zsolt Borbely
kling: review-
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (614.02 KB, application/zip)
2014-04-02 02:15 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion (598.80 KB, application/zip)
2014-04-02 02:45 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (565.97 KB, application/zip)
2014-04-02 03:42 PDT, Build Bot
no flags Details
Proposed patch (2.79 KB, patch)
2014-05-19 07:13 PDT, Zsolt Borbely
darin: review-
darin: commit-queue-
Details | Formatted Diff | Diff
Proposed patch (2.18 KB, patch)
2014-05-21 13:49 PDT, Zsolt Borbely
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zsolt Borbely 2014-04-02 01:21:49 PDT
Remove one of the CSSProperty constructor, because it is obsolete.
Comment 1 Zsolt Borbely 2014-04-02 01:24:10 PDT
Created attachment 228373 [details]
Proposed patch
Comment 2 Build Bot 2014-04-02 02:15:41 PDT
Comment on attachment 228373 [details]
Proposed patch

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

New failing tests:
fast/inspector-support/style.html
platform/mac/fast/scrolling/scroll-iframe-latched-mainframe.html
fast/css/duplicate-property-in-rule-important.html
fast/css/font-property-priority.html
platform/mac/fast/scrolling/scroll-select-latched-mainframe.html
fast/css/remove-shorthand.html
fast/css/url-with-multi-byte-unicode-escape.html
fast/css/important-js-override.html
fast/backgrounds/repeat/parsing-background-repeat.html
fast/css/cssText-shorthand.html
fast/css/overflow-property.html
fast/dom/background-shorthand-csstext.html
platform/mac/fast/scrolling/scroll-div-latched-mainframe.html
fast/css/webkit-mask-crash-implicit.html
Comment 3 Build Bot 2014-04-02 02:15:44 PDT
Created attachment 228375 [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.5
Comment 4 Build Bot 2014-04-02 02:45:42 PDT
Comment on attachment 228373 [details]
Proposed patch

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

New failing tests:
fast/inspector-support/style.html
fast/css/duplicate-property-in-rule-important.html
fast/css/font-property-priority.html
fast/css/remove-shorthand.html
fast/css/url-with-multi-byte-unicode-escape.html
fast/css/important-js-override.html
fast/backgrounds/repeat/parsing-background-repeat.html
fast/css/cssText-shorthand.html
fast/css/overflow-property.html
fast/dom/background-shorthand-csstext.html
fast/css/webkit-mask-crash-implicit.html
Comment 5 Build Bot 2014-04-02 02:45:45 PDT
Created attachment 228379 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 6 Build Bot 2014-04-02 03:42:44 PDT
Comment on attachment 228373 [details]
Proposed patch

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

New failing tests:
fast/inspector-support/style.html
fast/css/duplicate-property-in-rule-important.html
fast/css/font-property-priority.html
fast/css/remove-shorthand.html
fast/css/url-with-multi-byte-unicode-escape.html
fast/css/important-js-override.html
fast/backgrounds/repeat/parsing-background-repeat.html
fast/css/cssText-shorthand.html
fast/css/overflow-property.html
fast/dom/background-shorthand-csstext.html
fast/css/webkit-mask-crash-implicit.html
Comment 7 Build Bot 2014-04-02 03:42:47 PDT
Created attachment 228380 [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.5
Comment 8 Andreas Kling 2014-04-02 12:08:36 PDT
Comment on attachment 228373 [details]
Proposed patch

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

> Source/WebCore/css/StyleProperties.h:78
> -        CSSProperty toCSSProperty() const { return CSSProperty(propertyMetadata(), const_cast<CSSValue*>(propertyValue())); }
> +        CSSProperty toCSSProperty() const { return CSSProperty(id(), const_cast<CSSValue*>(propertyValue())); }

Many test failures here.
CSSProperty(id, value) is not the same as CSSProperty(metadata, value).
Comment 9 Zsolt Borbely 2014-05-19 07:13:39 PDT
Created attachment 231687 [details]
Proposed patch
Comment 10 Darin Adler 2014-05-21 09:42:38 PDT
Comment on attachment 231687 [details]
Proposed patch

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

> Source/WebCore/css/StyleProperties.h:70
> +        bool isSetFromShorthand() const { return propertyMetadata().m_isSetFromShorthand; }
> +        int indexInShorthandsVector() const { return propertyMetadata().m_indexInShorthandsVector; }

Please don’t add these new public member functions. We don’t need the functions at all, since they are only used in one place and that’s inside this class. And if you really felt compelled to add the functions, they should be private since they are only used by a member function.
Comment 11 Zsolt Borbely 2014-05-21 13:49:05 PDT
Created attachment 231841 [details]
Proposed patch

You are right, the patch is updated.
Comment 12 WebKit Commit Bot 2014-05-24 20:12:57 PDT
Comment on attachment 231841 [details]
Proposed patch

Clearing flags on attachment: 231841

Committed r169314: <http://trac.webkit.org/changeset/169314>
Comment 13 WebKit Commit Bot 2014-05-24 20:13:03 PDT
All reviewed patches have been landed.  Closing bug.