Bug 27880

Summary:

HTML 5: Support click() method on HTMLElement

Product:

WebKit

Reporter:

Antoine Quint <ml>

Component:

DOM

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, andersca, arko, bharathwaaj.s, bugmail, cmarcelo, darin, ericbidelman, eric, ian, ojan, paulirish, sam, timothy, tkent, tonikitoo, webkit.review.bot

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Bug Depends on:

Bug Blocks:

65215

Attachments:

Description	Flags
Proposed patch	none
Updated patch	abarth: review+
Updated patch	none
Updated patch	none
Updated patch	timothy: review+, timothy: commit-queue-
Updated patch	none

Antoine Quint

Reported 2009-07-31 07:48:48 PDT

The HTML 5 spec adds the click() method on all HTML elements, WebKit does not support it yet.

Attachments
Proposed patch (10.73 KB, patch) 2011-11-17 05:37 PST, Arko Saha	no flags	Details Formatted Diff Diff
Updated patch (12.15 KB, patch) 2011-11-18 02:32 PST, Arko Saha	abarth: review+	Details Formatted Diff Diff
Updated patch (14.38 KB, patch) 2012-02-11 01:11 PST, Arko Saha	no flags	Details Formatted Diff Diff
Updated patch (11.79 KB, patch) 2012-02-11 03:25 PST, Arko Saha	no flags	Details Formatted Diff Diff
Updated patch (12.49 KB, patch) 2012-02-11 04:03 PST, Arko Saha	timothy: review+ timothy: commit-queue-	Details Formatted Diff Diff
Updated patch (11.81 KB, patch) 2012-02-11 09:07 PST, Arko Saha	no flags	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Adam Roben (:aroben)

Comment 1 2009-07-31 07:50:54 PDT

<rdar://problem/7108579>

Arko Saha

Comment 2 2011-11-17 05:37:59 PST

Created attachment 115571 [details] Proposed patch

Adam Barth

Comment 3 2011-11-17 10:23:47 PST

Comment on attachment 115571 [details] Proposed patch Does this patch build on Mac? I'm wondering if we need to change the public ObjC interfaces header.

Arko Saha

Comment 4 2011-11-18 02:32:25 PST

Created attachment 115771 [details] Updated patch Incorporating review comments. Also verified the patch on apple-mac port.

Adam Barth

Comment 5 2011-11-18 10:31:25 PST

Comment on attachment 115771 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=115771&action=review > Source/WebCore/bindings/objc/PublicDOMInterfaces.h:-421 > -- (void)click AVAILABLE_WEBKIT_VERSION_3_0_AND_LATER; Looks like we lost the version attribute. Do we need to keep it?

Adam Barth

Comment 6 2011-11-18 10:32:21 PST

Comment on attachment 115771 [details] Updated patch This change looks good to me. Please double-check with some Apple folks about the version attribute in the public ObjC DOM bindings.

Arko Saha

Comment 7 2011-11-18 12:04:57 PST

Hi Sam/Darin, Can you please check the patch once. Here I removed the click() method of HTMLButton element from the public ObjC DOM bindings. It has a version attribute. Can you please check do we need to keep it?

Eric Seidel (no email)

Comment 8 2011-12-21 15:34:35 PST

xenon has historically been arbiter of the obj-c bindings.

Timothy Hatcher

Comment 9 2011-12-21 15:40:30 PST

Comment on attachment 115771 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=115771&action=review >> Source/WebCore/bindings/objc/PublicDOMInterfaces.h:-421 >> -- (void)click AVAILABLE_WEBKIT_VERSION_3_0_AND_LATER; > > Looks like we lost the version attribute. Do we need to keep it? This needs a new availability macro. And click likely should stay on the old classes with the old macro. This same thing came up with accessKey. See: https://bugs.webkit.org/show_bug.cgi?id=72756

Arko Saha

Comment 10 2012-02-11 01:11:36 PST

Created attachment 126624 [details] Updated patch

WebKit Review Bot

Comment 11 2012-02-11 01:13:56 PST

Please wait for approval from timothy@apple.com (or another member of the Apple Safari Team) before submitting because this patch contains changes to the Apple Mac WebKit.framework public API.

Eric Seidel (no email)

Comment 12 2012-02-11 01:21:28 PST

Comment on attachment 126624 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=126624&action=review > Source/WebCore/bindings/objc/PublicDOMInterfaces.h:421 > +- (void)click AVAILABLE_WEBKIT_VERSION_3_0_AND_LATER_BUT_DEPRECATED_AFTER_WEBKIT_VERSION_5_1; Are you sure you have to do this this way? I doubt that any Obj-c program cares if -click is exposed on these subclasses or on the baseclass. Since you're adding it to HTMLElement, things should "just work" and I would expect we could remove this w/o deprecation warnings.

Arko Saha

Comment 13 2012-02-11 01:38:27 PST

Thanks Eric for the review. (In reply to comment #12) > (From update of attachment 126624 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=126624&action=review > > > Source/WebCore/bindings/objc/PublicDOMInterfaces.h:421 > > +- (void)click AVAILABLE_WEBKIT_VERSION_3_0_AND_LATER_BUT_DEPRECATED_AFTER_WEBKIT_VERSION_5_1; > > Are you sure you have to do this this way? I doubt that any Obj-c program cares if -click is exposed on these subclasses or on the baseclass. Since you're adding it to HTMLElement, things should "just work" and I would expect we could remove this w/o deprecation warnings. Ok, so if we keep the -click in HTMLButtonElement and HTMLInputElement with old macro will it work on mac? Do we need to keep the method under LANGUAGE_OBJECTIVE_C in .idl files.

Arko Saha

Comment 14 2012-02-11 03:25:25 PST

Created attachment 126625 [details] Updated patch Incorporating Eric's review comments.

Arko Saha

Comment 15 2012-02-11 04:03:06 PST

Created attachment 126626 [details] Updated patch Resolving Mac build error.

Timothy Hatcher

Comment 16 2012-02-11 07:17:12 PST

Comment on attachment 126626 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=126626&action=review I supose you should do the same for accessKey then? > LayoutTests/ChangeLog:54 > - * platform/chromium-mac-snowleopard/svg/filters/filter-refresh-expected.txt: > + *youtTests/ChangeLog platform/chromium-mac-snowleopard/svg/filters/filter-refresh-expected.txt: Please fix this before landing.

Arko Saha

Comment 17 2012-02-11 09:07:05 PST

Created attachment 126637 [details] Updated patch

Arko Saha

Comment 18 2012-02-11 09:09:42 PST

Thanks Timothy for the review. (In reply to comment #16) > (From update of attachment 126626 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=126626&action=review > > I supose you should do the same for accessKey then? Sure, I will fix the same for accessKey. Shall I log a new bug for this? > > LayoutTests/ChangeLog:54 > > - * platform/chromium-mac-snowleopard/svg/filters/filter-refresh-expected.txt: > > + *youtTests/ChangeLog platform/chromium-mac-snowleopard/svg/filters/filter-refresh-expected.txt: > > Please fix this before landing. Done!

Timothy Hatcher

Comment 19 2012-02-11 09:19:13 PST

(In reply to comment #18) > Thanks Timothy for the review. > > (In reply to comment #16) > > (From update of attachment 126626 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=126626&action=review > > > > I supose you should do the same for accessKey then? > > Sure, I will fix the same for accessKey. Shall I log a new bug for this? The old bug is fine.

WebKit Review Bot

Comment 20 2012-02-11 10:52:09 PST

Comment on attachment 126637 [details] Updated patch Clearing flags on attachment: 126637 Committed r107483: <http://trac.webkit.org/changeset/107483>

WebKit Review Bot

Comment 21 2012-02-11 10:52:17 PST

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.