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

Kent Tamura

Reported 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.

Attachments
Patch 1 (11.86 KB, patch) 2012-09-10 01:21 PDT, Kent Tamura	no flags	Details Formatted Diff Diff
Patch 2 (19.69 KB, patch) 2012-09-11 19:15 PDT, Kent Tamura	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Kent Tamura

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

Created attachment 163062 [details] Patch 1

Hajime Morrita

Comment 2 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.

Kent Tamura

Comment 3 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.

Hajime Morrita

Comment 4 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.

Kent Tamura

Comment 5 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.

Kent Tamura

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

Created attachment 163496 [details] Patch 2

Hajime Morrita

Comment 7 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.

WebKit Review Bot

Comment 8 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>

WebKit Review Bot

Comment 9 2012-09-12 18:02:13 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.