[webkit-reviews] review granted: [Bug 118465] Update the HTML Media Capture implementation. : [Attachment 223860] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Raphael Kubo da Costa (:rakuco)
<rakuco at webkit.org>'s request for review:
Bug 118465: Update the HTML Media Capture implementation.
https://bugs.webkit.org/show_bug.cgi?id=118465

Attachment 223860: Patch
https://bugs.webkit.org/attachment.cgi?id=223860&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=223860&action=review


> Source/WebCore/html/HTMLInputElement.cpp:1806
> -    if (!isFileUpload())
> -	   return String();
> -
> -    String capture = fastGetAttribute(captureAttr).lower();
> -    if (capture == "camera"
> -	   || capture == "camcorder"
> -	   || capture == "microphone"
> -	   || capture == "filesystem")
> -	   return capture;
> -
> -    return "filesystem";
> -}
> +    if (!isFileUpload() || !fastHasAttribute(captureAttr))
> +	   return false;
>  
> -void HTMLInputElement::setCapture(const String& value)
> -{
> -    setAttribute(captureAttr, value);
> +    return true;

Coding style wise, the the way to write this is:

    return isFileUpload() && fastHasAttribute(captureAttr);

But getting at capture from the DOM API in JavaScript or other bindings will
give true, even if isFileUpload() is false.

If we keep this function, I suggest we name it differently to make it clear
it’s not exactly the same as the DOM API function.

> Source/WebCore/html/HTMLInputElement.h:301
> +    bool capture() const;

As I said above, I think it’s not good to have this HTMLInputElement::capture
function that returns something different from what calling capture on an
HTMLInputElement from JavaScript would give. It should be named differently.