[webkit-reviews] review granted: [Bug 201458] [iOS] Disabled options in the multi-select picker should not be selectable : [Attachment 407321] Patch

[bugzilla-daemon at webkit.org]

Wenson Hsieh <wenson_hsieh at apple.com> has granted Aditya Keerthi
<akeerthi at apple.com>'s request for review:
Bug 201458: [iOS] Disabled options in the multi-select picker should not be
selectable
https://bugs.webkit.org/show_bug.cgi?id=201458

Attachment 407321: Patch

https://bugs.webkit.org/attachment.cgi?id=407321&action=review




--- Comment #3 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 407321
  --> https://bugs.webkit.org/attachment.cgi?id=407321
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=407321&action=review

> Source/WebKit/UIProcess/ios/forms/WKFormSelectPicker.mm:289
> +	   [[view titleLabel] setTextColor:[UIColor colorWithWhite:0.0
alpha:(item.isGroup) ? GroupOptionTextColorAlpha : DisabledOptionAlpha]];

Nit - I think it's a little cleaner without parentheses around item.isGroup
here.

> LayoutTests/fast/forms/ios/disabled-options-in-multi-select-picker.html:53
> +    if (!selectElement.multiple) {

Nit - we usually avoid braces around single-line if statements, even in
JavaScript.

> LayoutTests/fast/forms/ios/disabled-options-in-multi-select-picker.html:61
> +    for (i = 0; i < element.getElementsByTagName("*").length; i++) {

(Ditto)