Introducing new API placeHolder() to expose placeholder attribute on MAC

Introducing new API placeHolder() to expose placeholder attribute on MAC.

On mac, placeholder attr has to be exposed as AXPlaceholder : <value>

BUG=440829
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=187623

[shreeramk, 2014-12-18 04:05:29]

shreeram.k@samsung.com changed reviewers:
+ aboxhall@chromium.org

[shreeramk, 2014-12-18 04:05:30]

Please review.
Thanks!!

[shreeramk, 2014-12-18 04:05:41]

shreeram.k@samsung.com changed reviewers:
+ dmazzoni@chromium.org

[aboxhall, 2014-12-18 04:17:54]

https://codereview.chromium.org/813473004/diff/20001/Source/modules/accessibi...
File Source/modules/accessibility/AXNodeObject.cpp (right):

https://codereview.chromium.org/813473004/diff/20001/Source/modules/accessibi...
Source/modules/accessibility/AXNodeObject.cpp:971: if
(hasAttribute(placeholderAttr))
This should be something like:

HTMLInputElement* inputElement = toHTMLInputElement(node);
String placeholder = inputElement->strippedPlaceholder();
if (!placeholder.isEmpty())
    return placeholder;

This way you're guaranteed to only get a placeholder string when the attribute
is supported (consider <input type="range"> for example).

https://codereview.chromium.org/813473004/diff/20001/Source/modules/accessibi...
File Source/modules/accessibility/AXNodeObject.h (right):

https://codereview.chromium.org/813473004/diff/20001/Source/modules/accessibi...
Source/modules/accessibility/AXNodeObject.h:78: String placeHolder() const;
This should be all lower-case (placeholder) as it is a single word.

[shreeramk, 2014-12-18 10:01:50]

shreeram.k@samsung.com changed reviewers:
+ mkwst@chromium.org

[shreeramk, 2014-12-18 10:01:51]

mkwst@chromium.org: Please review changes in web/

[shreeramk, 2014-12-18 16:54:00]

@aboxhall: PTAL. Thanks!


On 2014/12/18 04:17:54, aboxhall wrote:
>
https://codereview.chromium.org/813473004/diff/20001/Source/modules/accessibi...
> File Source/modules/accessibility/AXNodeObject.cpp (right):
> 
>
https://codereview.chromium.org/813473004/diff/20001/Source/modules/accessibi...
> Source/modules/accessibility/AXNodeObject.cpp:971: if
> (hasAttribute(placeholderAttr))
> This should be something like:
> 
> HTMLInputElement* inputElement = toHTMLInputElement(node);
> String placeholder = inputElement->strippedPlaceholder();
> if (!placeholder.isEmpty())
>     return placeholder;
> 
> This way you're guaranteed to only get a placeholder string when the attribute
> is supported (consider <input type="range"> for example).
> 
>
https://codereview.chromium.org/813473004/diff/20001/Source/modules/accessibi...
> File Source/modules/accessibility/AXNodeObject.h (right):
> 
>
https://codereview.chromium.org/813473004/diff/20001/Source/modules/accessibi...
> Source/modules/accessibility/AXNodeObject.h:78: String placeHolder() const;
> This should be all lower-case (placeholder) as it is a single word.

Done!

[shreeramk, 2014-12-22 03:33:19]

@dominic & @aboxhall: PTAL! Thanks.

[dmazzoni, 2014-12-22 08:03:26]

dmazzoni@chromium.org changed reviewers:
+ rbyers@chromium.org
- mkwst@chromium.org

[dmazzoni, 2014-12-22 08:03:27]

lgtm

+rbyers for OWNERS review of public/

https://codereview.chromium.org/813473004/diff/40001/Source/modules/accessibi...
File Source/modules/accessibility/AXNodeObject.cpp (right):

https://codereview.chromium.org/813473004/diff/40001/Source/modules/accessibi...
Source/modules/accessibility/AXNodeObject.cpp:970: String placeholder =
String();
The = String() isn't needed; just "String placeholder;" does the same thing.

[shreeramk, 2014-12-22 10:05:47]

The CQ bit was checked by shreeram.k@samsung.com

[commit-bot: I haz the power, 2014-12-22 10:06:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/813473004/60001

[commit-bot: I haz the power, 2014-12-22 10:12:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-22 10:12:57]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/2...)

[pdr., 2014-12-22 17:54:55]

pdr@chromium.org changed reviewers:
+ pdr@chromium.org

[pdr., 2014-12-22 17:54:55]

The CQ bit was checked by pdr@chromium.org

[pdr., 2014-12-22 17:54:56]

lgtm

[commit-bot: I haz the power, 2014-12-22 17:55:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/813473004/60001

[commit-bot: I haz the power, 2014-12-22 17:58:56]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=187623