Bug 78111 - Add unprefixed Blob.webkitSlice (slice)
: Add unprefixed Blob.webkitSlice (slice)
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: New Bugs
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To: Kinuko Yasuda
: WebExposed
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-08 06:08 PST by Simon Pieters
Modified: 2012-06-12 23:51 PDT (History)
12 users (show)

See Also:


Attachments
Patch (16.99 KB, patch)
2012-06-11 04:09 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (18.85 KB, patch)
2012-06-11 04:45 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (20.00 KB, patch)
2012-06-11 05:51 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (20.00 KB, patch)
2012-06-11 06:14 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (20.01 KB, patch)
2012-06-11 06:50 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-04 (1.22 MB, application/zip)
2012-06-11 09:38 PDT, WebKit Review Bot
no flags Details
Patch (20.45 KB, patch)
2012-06-11 23:51 PDT, Kinuko Yasuda
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Pieters 2012-02-08 06:08:07 PST
Please rename Blob.webkitSlice to Blob.slice. Opera is working on changing the semantics of Blob.slice, but we don't want to prefix it. Since Gecko and WebKit have been shipping without Blob.slice support for several months, the existing content, if any, that relied on the old semantics of Blob.slice have had the chance to break and get updated. With Opera soon shipping Blob.slice with the new semantics, it's time to drop the prefix.
Comment 1 Kinuko Yasuda 2012-06-11 04:09:53 PDT
Created attachment 146823 [details]
Patch
Comment 2 Gustavo Noronha (kov) 2012-06-11 04:23:32 PDT
Comment on attachment 146823 [details]
Patch

Attachment 146823 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12943214
Comment 3 Kinuko Yasuda 2012-06-11 04:45:05 PDT
Created attachment 146833 [details]
Patch
Comment 4 Kinuko Yasuda 2012-06-11 05:51:07 PDT
Created attachment 146839 [details]
Patch
Comment 5 Kinuko Yasuda 2012-06-11 06:14:56 PDT
Created attachment 146843 [details]
Patch
Comment 6 Gustavo Noronha (kov) 2012-06-11 06:29:21 PDT
Comment on attachment 146843 [details]
Patch

Attachment 146843 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12941324
Comment 7 Kinuko Yasuda 2012-06-11 06:50:59 PDT
Created attachment 146848 [details]
Patch
Comment 8 WebKit Review Bot 2012-06-11 09:38:39 PDT
Comment on attachment 146848 [details]
Patch

Attachment 146848 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12941361

New failing tests:
fast/filesystem/workers/file-writer-truncate-extend.html
fast/filesystem/workers/file-writer-write-overlapped.html
fast/filesystem/file-writer-truncate-extend.html
inspector/profiler/heap-snapshot-loader.html
Comment 9 WebKit Review Bot 2012-06-11 09:38:44 PDT
Created attachment 146867 [details]
Archive of layout-test-results from ec2-cr-linux-04

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-04  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 10 Kinuko Yasuda 2012-06-11 23:51:30 PDT
Created attachment 147024 [details]
Patch
Comment 11 Kinuko Yasuda 2012-06-12 21:05:01 PDT
Could someone review this?
Comment 12 Adam Barth 2012-06-12 21:08:51 PDT
Comment on attachment 147024 [details]
Patch

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

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:32
> -WEBKIT_API WebKitDOMBlob* webkit_dom_blob_slice(WebKitDOMBlob* self, gint64 start, gint64 end, const gchar* content_type);
> +WEBKIT_API WebKitDOMBlob* webkit_dom_blob_webkit_slice(WebKitDOMBlob* self, gint64 start, gint64 end, const gchar* content_type);

Why the addition of "webkit" here?  I would have thought we'd be happy with fewer webkits, not more.

> Source/WebCore/fileapi/Blob.cpp:97
> +    String message("Blob.webkitSlice() is deprecated. Use Blob.slice() instead .");

Looks like you've got an extra space before the .

> Source/WebCore/fileapi/Blob.h:75
> +    // Prefixed version is going to be deprecated. This internally calls sliceInternal() (as slice() does) after showing a deprecation message.

"is going to be deprecated" => "is deprecated" :)
Comment 13 Kinuko Yasuda 2012-06-12 21:14:48 PDT
(In reply to comment #12)
> (From update of attachment 147024 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=147024&action=review
> 
> > Source/WebCore/bindings/gobject/WebKitDOMCustom.h:32
> > -WEBKIT_API WebKitDOMBlob* webkit_dom_blob_slice(WebKitDOMBlob* self, gint64 start, gint64 end, const gchar* content_type);
> > +WEBKIT_API WebKitDOMBlob* webkit_dom_blob_webkit_slice(WebKitDOMBlob* self, gint64 start, gint64 end, const gchar* content_type);
> 
> Why the addition of "webkit" here?  I would have thought we'd be happy with fewer webkits, not more.

It's for keeping backward compatibility, because I added !LANGUAGE_GOBJECT not to generate webkitSlice binding for gobject as its binding doesn't support CallWith=ScriptExecutionContext. (Hope this works for gtk)

> > Source/WebCore/fileapi/Blob.cpp:97
> > +    String message("Blob.webkitSlice() is deprecated. Use Blob.slice() instead .");
> 
> Looks like you've got an extra space before the .
> 
> > Source/WebCore/fileapi/Blob.h:75
> > +    // Prefixed version is going to be deprecated. This internally calls sliceInternal() (as slice() does) after showing a deprecation message.
> 
> "is going to be deprecated" => "is deprecated" :)

Will fix these nits.  Thanks for your review!
Comment 14 Adam Barth 2012-06-12 21:22:09 PDT
Ah!  I understand.  Thanks.  :)
Comment 15 Kinuko Yasuda 2012-06-12 23:51:04 PDT
Committed r120165: <http://trac.webkit.org/changeset/120165>