[webkit-reviews] review denied: [Bug 72756] REGRESSION (r100805):=?UTF-8?Q?=20DOMHTMLElement=E2=80=99s=20accessKey=20property=20is=20declared=20as=20available=20in=20WebKit=20version=20that=20didn=E2=80=99t=20have=20it=20?=: [Attachment 126506] proposed patch

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at apple.com> has denied Vineet Chaudhary (vineetc)
<rgf748 at motorola.com>'s request for review:
Bug 72756: REGRESSION (r100805): DOMHTMLElement’s accessKey property is
declared as available in WebKit version that didn’t have it
https://bugs.webkit.org/show_bug.cgi?id=72756

Attachment 126506: proposed patch
https://bugs.webkit.org/attachment.cgi?id=126506&action=review

------- Additional Comments from Timothy Hatcher <timothy at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=126506&action=review


r- for the wrong versions in the macros.

> Source/JavaScriptCore/API/WebKitAvailability.h:916
> + * AVAILABLE_WEBKIT_VERSION_5_1_AND_LATER
> + * 
> + * Used on functions introduced in WebKit 5.1
> + */
> +#define AVAILABLE_WEBKIT_VERSION_5_1_AND_LATER

Safari/WebKit 5.1 has already shipped. So this is not the right version.

AVAILABLE_AFTER__WEBKIT_VERSION_5_1?

Mark, thoughts?

> Source/WebCore/bindings/objc/PublicDOMInterfaces.h:334
> + at property(copy) NSString *accessKey
AVAILABLE_WEBKIT_VERSION_1_3_AND_LATER_BUT_DEPRECATED_AFTER_WEBKIT_VERSION_4_0;


This (and all other uses you added) should be
AVAILABLE_WEBKIT_VERSION_1_3_AND_LATER_BUT_DEPRECATED_AFTER_WEBKIT_VERSION_5_1.
So you need to add that new macro. Because version 5 and 5.1 of WebKit had
these.

> Source/WebCore/html/HTMLInputElement.idl:56
>  #if defined(LANGUAGE_OBJECTIVE_C) && LANGUAGE_OBJECTIVE_C
> +	   attribute [Reflect] DOMString accessKey;
> +#endif
> +#if defined(LANGUAGE_OBJECTIVE_C) && LANGUAGE_OBJECTIVE_C
>	   attribute [ObjCImplementedAsUnsignedLong] DOMString size; // DOM
level 2 changed this to a long, but ObjC API is a string

You can group these #ifdefs.