Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(120)

Issue 2641223003: Proof of concept that CSSPropertyAPI*::parseSingleValue may need more tahn 2 arguments (Closed)

Created:
3 years, 11 months ago by nainar
Modified:
3 years, 10 months ago
Reviewers:
sashab, aazzam
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, chromium-reviews, dglazkov+blink, rwlbuis
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Proof of concept that CSSPropertyAPI*::parseSingleValue may need more tahn 2 arguments This patch is just to show a proof of concept that the h file generator for CSSPropertyAPI* will need to incorporate the fact that CSSPropertyAPI*::parseSingleValue has three input arguments and not two.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+47 lines, -54 lines) Patch
M third_party/WebKit/Source/core/css/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSProperties.in View 4 chunks +18 lines, -18 lines 0 comments Download
M third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp View 2 chunks +0 lines, -36 lines 0 comments Download
A third_party/WebKit/Source/core/css/properties/CSSPropertyAPIBackgroundComponent.cpp View 1 chunk +28 lines, -0 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 3 (1 generated)
nainar
Hi Anna, Sasha I was working on a patch to move code for BackgroundComponents over ...
3 years, 11 months ago (2017-01-20 08:18:31 UTC) #2
aazzam
3 years, 11 months ago (2017-01-20 22:53:35 UTC) #3
On 2017/01/20 at 08:18:31, nainar wrote:
> Hi Anna, Sasha 
> 
> I was working on a patch to move code for BackgroundComponents over to
CSSPropertyAPIBackgroundComponent. And seems like the template for
parseSingleValue only incorporates CSSParserTokenRange& range, const
CSSParserContext* context. Whereas in this case CSSPropertyID
unresolvedProperty, CSSParserTokenRange& range, const CSSParserContext* context
are the input arguments passed. 
> 
> I think either the template needs to be changed or this case needs to forego
the template. Let me know if you need me to explain this to you IRL. 
> 
> Thanks!

hey naina!! thanks for taking a look at this :) if you look at my design doc,
you'll see that i've decided that for the (few) properties where unresolved
properties are needed, the alias should be implemented for these properties. if
there is an eventual aim to remove CSSPropertyID's from the codebase, it's
probably best that we dont' pass this in to the API. 

here's my design doc, have a look at the "aliases and shorthand properties"
section, and let me know your thoughts :)
https://docs.google.com/document/d/1R2qiaQJ-SKkTTvnoUkuT9f-dmk5GlhzcvzeQj3iQa...

Powered by Google App Engine
This is Rietveld 408576698