Summary: | [Forms] Introduce runtime feature flags for input type datetime, datetimelocal, month, time, week | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | yosin | ||||||||||
Component: | Forms | Assignee: | yosin | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | abarth, dglazkov, fishd, jamesr, mifenton, tkent, tkent+wkapi, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 88970 | ||||||||||||
Attachments: |
|
Description
yosin
2012-07-25 23:31:41 PDT
Created attachment 154585 [details]
Patch 1
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI. Comment on attachment 154585 [details]
Patch 1
WebKit API change LGTM. Please be sure to disable these features appropriately on chromium-android.
Comment on attachment 154585 [details] Patch 1 View in context: https://bugs.webkit.org/attachment.cgi?id=154585&action=review > Source/WebCore/bindings/generic/RuntimeEnabledFeatures.cpp:202 > +#if OS(ANDROID) Is this the best way to do this? I would have expected the embedder to do this work. (In reply to comment #4) > (From update of attachment 154585 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=154585&action=review > > > Source/WebCore/bindings/generic/RuntimeEnabledFeatures.cpp:202 > > +#if OS(ANDROID) > > Is this the best way to do this? I would have expected the embedder to do this work. No, this isn't beautiful. We should not have this kind of #if in RuntimeEnabledFeatures.cpp I'll do following three steps to make RuntimeEnabledFeatures.cpp clean: 1. RuntimEnabledFeatures.cpp: Change #if OS(ANDROID) to #if PLATFORM(CHROMIUM) && !OS(ANDROID) to set false, otherwise set true -- keep current behavior 2. content/renderrer/render_thread_impl.cc: RenderThreadImpl::EnsureWebKitInitialized() of Chromium side, I'll add enableInputTypeDateTime(false), ... for !OS(ANDROID) 3. Then, remove #if PLATFORM(CHROMIUM) && !OS(ANDROID) from RuntimeEnabledFeatures.cpp Comment on attachment 154585 [details]
Patch 1
Please touch InputType.h to rebuild *InputType.cpp. Without it, Chromium-win bots will fail to build.
Created attachment 154592 [details]
Patch 2
Created attachment 154597 [details]
Patch 3
> I'll do following three steps to make RuntimeEnabledFeatures.cpp clean:
That sounds great. Thanks@
Comment on attachment 154597 [details]
Patch 3
Could you review this patch?
Thanks in advance.
Comment on attachment 154597 [details] Patch 3 View in context: https://bugs.webkit.org/attachment.cgi?id=154597&action=review The code looks ok. > Source/WebCore/ChangeLog:3 > + [Forms] Introduce runtime feature flags for input type datetime, datetimelocal, month, time, week for Chromium port Flags will be introduced to WebCore. "for Chromium port" is false. Created attachment 154804 [details]
Patch 4
Comment on attachment 154804 [details] Patch 4 Clearing flags on attachment: 154804 Committed r123826: <http://trac.webkit.org/changeset/123826> All reviewed patches have been landed. Closing bug. |