[webkit-reviews] review denied: [Bug 53776] Device element patch 1: adding a basic skeleton of the HTMLDeviceElement and the Stream objects. : [Attachment 81242] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Feb 5 01:55:05 PST 2011
Eric Seidel <eric at webkit.org> has denied Leandro Graciá Gil
<leandrogracia at chromium.org>'s request for review:
Bug 53776: Device element patch 1: adding a basic skeleton of the
HTMLDeviceElement and the Stream objects.
https://bugs.webkit.org/show_bug.cgi?id=53776
Attachment 81242: Patch
https://bugs.webkit.org/attachment.cgi?id=81242&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=81242&action=review
build failures.
> Source/WebCore/dom/DeviceType.cpp:43
> +// Avoid using static Strings. Code should not run on WebKit load.
> +const char* DeviceType::s_names[] =
> +{
> + "invalid",
> + "media"
> +};
We've historically done this with AtomicStrings and an init() method. But in
this case for only 2 types, I'm not sure having an array actually buys us
anything besides complexity.
> Source/WebCore/dom/Stream.h:49
> + static PassRefPtr<Stream> create(HTMLDeviceElement*, const String& url);
Should these urls be strings or KURLs?
> Source/WebCore/html/HTMLDeviceElement.cpp:87
> + HTMLElement::defaultEventHandler(event);
> + return;
This could just be one line.
More information about the webkit-reviews
mailing list