Bug 153522 - window.atob() should ignore spaces in input
Summary: window.atob() should ignore spaces in input
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL: https://html.spec.whatwg.org/#dom-win...
Keywords: InRadar, WebExposed
Depends on:
Blocks:
 
Reported: 2016-01-26 16:13 PST by Chris Dumez
Modified: 2016-01-27 14:38 PST (History)
6 users (show)

See Also:


Attachments
WIP Patch (15.31 KB, patch)
2016-01-26 16:20 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (20.33 KB, patch)
2016-01-27 10:49 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (21.58 KB, patch)
2016-01-27 14:31 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (21.61 KB, patch)
2016-01-27 14:37 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2016-01-26 16:13:45 PST
window.atob() should ignore spaces in input as per:
https://html.spec.whatwg.org/#dom-windowbase64-atob (Step 3)

Currently, WebKit throws an exception and it is the only browser to do so. Firefox and Chrome behavior according to the specification.

This causes us to fail 10 checks in the following W3C HTML test:
http://w3c-test.org/html/webappapis/atob/base64.html
Comment 1 Radar WebKit Bug Importer 2016-01-26 16:14:24 PST
<rdar://problem/24357822>
Comment 2 Chris Dumez 2016-01-26 16:20:43 PST
Created attachment 269945 [details]
WIP Patch
Comment 3 Alexey Proskuryakov 2016-01-26 21:31:43 PST
I'm pretty sure that we have an old bug with a discussion of what a stupid idea this was :(
Comment 4 Chris Dumez 2016-01-26 21:51:43 PST
(In reply to comment #3)
> I'm pretty sure that we have an old bug with a discussion of what a stupid
> idea this was :(

I forgot about it but now that you mention it, it does ring a bell.

The thing is that the spec has not changed since, other browsers still strip whitespaces as well. It is now covered by the W3C web-platform-tests too.
Comment 5 Alexey Proskuryakov 2016-01-26 22:48:08 PST
bug 123830
Comment 6 Chris Dumez 2016-01-27 09:57:33 PST
(In reply to comment #5)
> bug 123830

The fact at this point is that other browsers and the specification have been stripping spaces for several years now. Not doing this in WebKit is a compatibility risk and I think we should do this.
Comment 7 Alexey Proskuryakov 2016-01-27 10:07:08 PST
I'm afraid so, yes. It will probably change back eventually though.
Comment 8 Chris Dumez 2016-01-27 10:49:52 PST
Created attachment 270009 [details]
Patch
Comment 9 Benjamin Poulain 2016-01-27 14:18:33 PST
Comment on attachment 270009 [details]
Patch

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

> Source/WTF/wtf/text/Base64.cpp:182
>  }

Don't forget to update the copyright on this file.

> LayoutTests/fast/dom/Window/atob-btoa-expected.txt:29
> +PASS window.atob(" YQ==") is "a"

Can you please add tests covering the error cases?

There are 3 "hadError = true;". Each should have a test.
Comment 10 Chris Dumez 2016-01-27 14:31:25 PST
Created attachment 270041 [details]
Patch
Comment 11 Chris Dumez 2016-01-27 14:37:21 PST
Created attachment 270044 [details]
Patch
Comment 12 Chris Dumez 2016-01-27 14:37:54 PST
Comment on attachment 270044 [details]
Patch

Clearing flags on attachment: 270044

Committed r195694: <http://trac.webkit.org/changeset/195694>
Comment 13 Chris Dumez 2016-01-27 14:38:00 PST
All reviewed patches have been landed.  Closing bug.