Summary: | Implement parsing for CSS will-change | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||
Component: | New Bugs | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | bdakin, dino, jonlee, koivisto, simon.fraser, thorton | ||||||
Priority: | P2 | Keywords: | WebExposed | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 147772 | ||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2015-08-14 23:30:40 PDT
Created attachment 259083 [details]
Patch
Created attachment 259084 [details]
Patch
Comment on attachment 259084 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259084&action=review This is the first time I've tried to review from an iPad. I hope it works. > Source/WebCore/rendering/style/WillChangeData.cpp:45 > +bool WillChangeData::containsContents() const Why don't we remember this as it is added/removed? > Source/WebCore/rendering/style/WillChangeData.h:60 > + CustomIdent, Why not call this property? What other idents could there be? If they are not properties then we'd probably make keywords for them like "contents" > LayoutTests/fast/css/will-change-parsing.html:32 > + shouldBe('testComputedStyle(";")', '"auto"'); You probably meant will-change:; but this is still good. > LayoutTests/fast/css/will-change-parsing.html:37 > + shouldBe('test("will-change: initial;")', '"initial"'); Need a test for will-change:will-change; > LayoutTests/fast/css/will-change-parsing.html:43 > + shouldBe('test("will-change: scroll-position, scroll-position;")', '"scroll-position, scroll-position"'); It's kind of silly that the computed style doesn't remove duplicates since they do not change behaviour for this property. > LayoutTests/fast/css/will-change-parsing.html:51 > + shouldBe('test("will-change: background, transform;")', '"background, transform"'); Need a test that is missing commas > > Source/WebCore/rendering/style/WillChangeData.cpp:45 > > +bool WillChangeData::containsContents() const > > Why don't we remember this as it is added/removed? I'll do that as I start to use them. > > Source/WebCore/rendering/style/WillChangeData.h:60 > > + CustomIdent, > > Why not call this property? What other idents could there be? If they are > not properties then we'd probably make keywords for them like "contents" Changed it. > > LayoutTests/fast/css/will-change-parsing.html:32 > > + shouldBe('testComputedStyle(";")', '"auto"'); > > You probably meant will-change:; but this is still good. Fixed. > > LayoutTests/fast/css/will-change-parsing.html:37 > > + shouldBe('test("will-change: initial;")', '"initial"'); > > Need a test for will-change:will-change; Done (amusingly, Firefox fails this). > > LayoutTests/fast/css/will-change-parsing.html:43 > > + shouldBe('test("will-change: scroll-position, scroll-position;")', '"scroll-position, scroll-position"'); > > It's kind of silly that the computed style doesn't remove duplicates since > they do not change behaviour for this property. Agreed, but following other UAs and the spec here. > > LayoutTests/fast/css/will-change-parsing.html:51 > > + shouldBe('test("will-change: background, transform;")', '"background, transform"'); > > Need a test that is missing commas Added. |