Bug 68618 - btoa() and atob() should stringify null to "null", not ""
Summary: btoa() and atob() should stringify null to "null", not ""
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Minor
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-22 07:16 PDT by Aryeh Gregor
Modified: 2017-07-18 08:30 PDT (History)
5 users (show)

See Also:


Attachments
Patch (4.38 KB, patch)
2011-12-06 16:10 PST, Alexander Færøy
no flags Details | Formatted Diff | Diff
Patch (4.67 KB, patch)
2011-12-07 02:14 PST, Alexander Færøy
eric: review-
haraken: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aryeh Gregor 2011-09-22 07:16:02 PDT
Mozilla bug: https://bugzilla.mozilla.org/show_bug.cgi?id=688443

Spec:

http://www.whatwg.org/specs/web-apps/current-work/multipage/webappapis.html#atob

It doesn't have [TreatNullAs=EmptyString], so it should be treated as "null".  No one does this currently for btoa() or atob() -- Firefox 8.0a2, Chrome 15 dev, and Opera 11.50 stringify to "".  (IE doesn't support the functions.)  But it seems like consensus is that browsers should change here, not the spec:

http://krijnhoetmer.nl/irc-logs/whatwg/20110922#l-230 (first two lines)
http://krijnhoetmer.nl/irc-logs/whatwg/20110922#l-554 (continuation)

zcorpan reports Opera has already changed, and Ms2ger says Gecko should change too.  I'll update my base64 tests at the W3C to test for this.
Comment 1 Alexander Færøy 2011-12-06 16:10:08 PST
Created attachment 118133 [details]
Patch
Comment 2 Adam Barth 2011-12-06 16:36:22 PST
Comment on attachment 118133 [details]
Patch

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

> Source/WebCore/page/DOMWindow.idl:252
> -        DOMString btoa(in [ConvertNullToNullString,Optional=CallWithDefaultValue] DOMString string)
> +        DOMString btoa(in [ConvertNullStringTo=Null,Optional=CallWithDefaultValue] DOMString string)

Can we just drop the attribute?  What's the default value?
Comment 3 Alexander Færøy 2011-12-07 02:14:21 PST
Created attachment 118190 [details]
Patch
Comment 4 Alexander Færøy 2011-12-07 02:15:29 PST
Firefox is currently throwing a TypeError with not enough arguments when atob() and btoa() is called with no arguments. My patch above should fix that as well.
Comment 5 Kentaro Hara 2012-01-25 18:21:27 PST
Comment on attachment 118190 [details]
Patch

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

The change looks OK to me.

> Source/WebCore/ChangeLog:5
> +

Would you add some explanation about this change? At least the link to the spec, please.

> LayoutTests/ChangeLog:5
> +

Would you add some explanation about this change? At least the link to the spec, please.

> LayoutTests/fast/dom/Window/atob-btoa.html:25
> +shouldBe('window.btoa(null)', '"bnVsbA=="');
>  shouldBe('window.btoa(undefined)', '"dW5kZWZpbmVk"');

Shall we add the following test cases?

    shouldBe('window.btoa("null")', '"bnVsbA=="');
    shouldBe('window.btoa("undefined")', '"dW5kZWZpbmVk"');

> LayoutTests/fast/dom/Window/atob-btoa.html:35
>  shouldThrow('window.atob()'); // 'undefined' 

Can you remove the comment "// 'undefined'"?

> LayoutTests/fast/dom/Window/atob-btoa.html:38
> +shouldBe('window.atob(null)', '"Âée"');
>  shouldThrow('window.atob(undefined)');

Shall we add the following test cases?

    shouldBe('window.atob("null")', '"Âée"');
    shouldThrow('window.atob("undefined")');
Comment 6 Eric Seidel (no email) 2012-02-13 10:05:36 PST
Comment on attachment 118190 [details]
Patch

r-, per Kentaro's comments.