[Webkit-unassigned] [Bug 63062] Add capture attribute for HTML Media Capture

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=63062





--- Comment #48 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2012-02-28 08:39:19 PST ---
(From update of attachment 129021)
View in context: https://bugs.webkit.org/attachment.cgi?id=129021&action=review

Just some minor comment while skimming the patch

> Source/WebCore/platform/FileChooser.cpp:59
> +MediaCaptureChooser* FileChooserClient::newMediaCaptureChooser(const FileChooserSettings& settings, MediaCaptureType device)

It almost looks like the name of the class doesn't fit anymore? ContentChooserClient?

> Source/WebCore/platform/FileChooser.cpp:72
> +    for (Vector<String>::const_iterator iterator = acceptMIMETypes.begin(); iterator != acceptMIMETypes.end(); ++iterator)

We normally call the iterator for "it" and put the end outside the look.

> Source/WebCore/platform/FileChooser.h:71
>      FileChooser* newFileChooser(const FileChooserSettings&);
>  
> +#if ENABLE(HTML_MEDIA_CAPTURE)
> +    MediaCaptureChooser* newMediaCaptureChooser(const FileChooserSettings&, MediaCaptureType);

Could the newFileChooser and newMediaCaptureChooser be merged? Would it make sense?

> Source/WebKit/chromium/src/ChromeClientImpl.cpp:730
> +void ChromeClientImpl::runCapturePanel(Frame*, PassRefPtr<MediaCaptureChooser>)

im not sure this is always a panel, so maybe runCaptureChooser makes more sense? Though that would mean we would have to rename the runFilePanel as well

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.