Bug 96238

Summary:

Introduce ValidationMessageClient

Product:

WebKit

Reporter:

Kent Tamura <tkent>

Component:

Forms

Assignee:

Kent Tamura <tkent>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dglazkov, japhet, jonlee, mifenton, morrita, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

96365

Bug Blocks:

95527

Attachments:

Description	Flags
Patch 1	none
Patch 2	none

Description Kent Tamura 2012-09-10 00:09:43 PDT

Add new function to ChromeClient to manage form validation messages, and use it.

The function will be wrapped with a compile-time flag.

Comment 1 Kent Tamura 2012-09-10 01:21:22 PDT

Created attachment 163062 [details]
Patch 1

Comment 2 Hajime Morrita 2012-09-10 10:58:43 PDT

Comment on attachment 163062 [details]
Patch 1

Can we make ValidationMessage class an abstract class or an interface and add this new mechanism as a subclass of it?
Also, could it be this new ChromeClient API a supplement?
Introducing new ifdefs are sad. It would be great if we can minimize its impact.

Comment 3 Kent Tamura 2012-09-10 18:46:10 PDT

(In reply to comment #2)
> (From update of attachment 163062 [details])
> Can we make ValidationMessage class an abstract class or an interface and add this new mechanism as a subclass of it?
> Also, could it be this new ChromeClient API a supplement?
> Introducing new ifdefs are sad. It would be great if we can minimize its impact.

I can move the changes into ValidationMessage class. However I object making a subclass and making the ChromeClient API a supplement. I'm going to remove the current implementation with Shadow DOM when we complete the ChromeClient API implementation for all platforms.

Comment 4 Hajime Morrita 2012-09-10 19:05:10 PDT

Comment on attachment 163062 [details]
Patch 1

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 163062 [details] [details])
> > Can we make ValidationMessage class an abstract class or an interface and add this new mechanism as a subclass of it?
> > Also, could it be this new ChromeClient API a supplement?
> > Introducing new ifdefs are sad. It would be great if we can minimize its impact.
> 
> I can move the changes into ValidationMessage class. However I object making a subclass and making the ChromeClient API a supplement. I'm going to remove the current implementation with Shadow DOM when we complete the ChromeClient API implementation for all platforms.

That makes sense.
What do you think about per-validation-message state tracking?
If WebKit layer needs to maintain element-to-state map, probably we could morph
ValidationMessage to ValidationMessageClient (or something) which is implemented by 
WebKit layer. Such object could also be used to notify relayout change, etc.

Comment 5 Kent Tamura 2012-09-11 03:30:43 PDT

(In reply to comment #4)
> What do you think about per-validation-message state tracking?
> If WebKit layer needs to maintain element-to-state map, probably we could morph
> ValidationMessage to ValidationMessageClient (or something) which is implemented by 
> WebKit layer. Such object could also be used to notify relayout change, etc.

The client approach sounds good.  It makes the patch cleaner.
I have posted a patch to prepare the client approach to Bug 96365.

Comment 6 Kent Tamura 2012-09-11 19:15:59 PDT

Created attachment 163496 [details]
Patch 2

Comment 7 Hajime Morrita 2012-09-12 09:26:38 PDT

Comment on attachment 163496 [details]
Patch 2

Looks good. ValidationMessageClient could be per-Element basis. But it might be overdone.

Comment 8 WebKit Review Bot 2012-09-12 18:02:08 PDT

Comment on attachment 163496 [details]
Patch 2

Clearing flags on attachment: 163496

Committed r128394: <http://trac.webkit.org/changeset/128394>

Comment 9 WebKit Review Bot 2012-09-12 18:02:13 PDT

All reviewed patches have been landed.  Closing bug.