Bug 101038 - BaseChooserOnlyDateAndTimeInputType should implement DateTimeChooserClient
Summary: BaseChooserOnlyDateAndTimeInputType should implement DateTimeChooserClient
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kent Tamura
URL:
Keywords:
Depends on: 101031
Blocks: 100935
  Show dependency tree / graph
 
Reported: 2012-11-02 03:25 PDT by Kent Tamura
Modified: 2012-11-05 00:22 PST (History)
5 users (show)

See Also:

Attachments
Patch (12.39 KB, patch)
2012-11-04 20:11 PST, Kent Tamura 		no flags Details | Formatted Diff | Diff
Patch 2 (12.43 KB, patch)
2012-11-04 20:40 PST, Kent Tamura 		morrita: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2012-11-02 03:25:15 PDT 
BaseChooserOnlyDateAndTimeInputType should implement DateTimeChooserClient
Comment 1 Kent Tamura 2012-11-04 20:11:33 PST 
Created attachment 172259 [details]
Patch
Comment 2 Peter Beverloo (cr-android ews) 2012-11-04 20:38:06 PST 
Comment on attachment 172259 [details]
Patch

Attachment 172259 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/14731445
Comment 3 Kent Tamura 2012-11-04 20:40:39 PST 
Created attachment 172262 [details]
Patch 2

Chromium-Android build fix
Comment 4 Hajime Morrita 2012-11-04 23:25:37 PST 
Comment on attachment 172262 [details]
Patch 2

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

> Source/WebCore/ChangeLog:15
> +        elements.

Is there any bug for this? Seems worth mentioning here.

> Source/WebCore/html/BaseChooserOnlyDateAndTimeInputType.cpp:44
>  {

Can we ASSERT() that this is attached()?
Just for curious, what happens if a DOMActivate event is sent to the element through JS?
Comment 5 Kent Tamura 2012-11-04 23:31:57 PST 
Comment on attachment 172262 [details]
Patch 2

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

>> Source/WebCore/ChangeLog:15
>> +        elements.
> 
> Is there any bug for this? Seems worth mentioning here.

will do.

>> Source/WebCore/html/BaseChooserOnlyDateAndTimeInputType.cpp:44
>>  {
> 
> Can we ASSERT() that this is attached()?
> Just for curious, what happens if a DOMActivate event is sent to the element through JS?

I don't think ASSERT(element()->attached()) is correct because, as you know, we can dispatch an event to a non-rendered element.
In such case, the ScriptController::processingUserGesture() check below rejects opening a DateTimeChooser.
Comment 6 Hajime Morrita 2012-11-04 23:44:32 PST 
Comment on attachment 172262 [details]
Patch 2

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

>>> Source/WebCore/html/BaseChooserOnlyDateAndTimeInputType.cpp:44
>>>  {
>> 
>> Can we ASSERT() that this is attached()?
>> Just for curious, what happens if a DOMActivate event is sent to the element through JS?
> 
> I don't think ASSERT(element()->attached()) is correct because, as you know, we can dispatch an event to a non-rendered element.
> In such case, the ScriptController::processingUserGesture() check below rejects opening a DateTimeChooser.

OK, thanks for the explanation.
Comment 7 Kent Tamura 2012-11-05 00:22:27 PST 
Committed r133441: <http://trac.webkit.org/changeset/133441>