Bug 9673 - Add support for window.atob() and window.btoa()
Summary: Add support for window.atob() and window.btoa()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Enhancement
Assignee: Alexey Proskuryakov
URL: http://developer.mozilla.org/en/docs/...
Keywords:
Depends on:
Blocks:
 
Reported: 2006-07-01 03:03 PDT by Alexey Proskuryakov
Modified: 2006-12-11 23:05 PST (History)
0 users

See Also:


Attachments
btoa test (24.68 KB, text/html)
2006-07-01 03:04 PDT, Alexey Proskuryakov
no flags Details
proposed fix (60.38 KB, patch)
2006-11-06 12:32 PST, Alexey Proskuryakov
sam: review-
Details | Formatted Diff | Diff
proposed fix (64.66 KB, patch)
2006-11-08 09:20 PST, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
now with unsigned instead of unsigned int (64.63 KB, patch)
2006-12-07 22:00 PST, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2006-07-01 03:03:56 PDT
These convert between base64 and binary strings, see the bug URL for documentation.

Supported by Mozilla and Konqueror, not supported by Opera 9, haven't tested WinIE.
Comment 1 Alexey Proskuryakov 2006-07-01 03:04:49 PDT
Created attachment 9117 [details]
btoa test
Comment 2 Alexey Proskuryakov 2006-11-06 12:32:57 PST
Created attachment 11405 [details]
proposed fix

This implementation is based on the one from KDE, but has been tweaked for better Firefox compatibility.

One thing I couldn't understand in khtml (and thus didn't copy) was a special case for AToB/BToA in Window::getOwnPropertySlot:

    switch(entry->value) {
    case Frames:
// ...
    case Blur:
    case AToB:
    case BToA:
      getSlotFromEntry<WindowFunc, Window>(entry, this, slot);

<http://websvn.kde.org/trunk/KDE/kdelibs/khtml/ecma/kjs_window.cpp?rev=592458&view=markup>

WebCore has similar (but not identical) code for Blur etc. here, so maybe we actually need it, too.
Comment 3 Sam Weinig 2006-11-08 07:11:42 PST
Comment on attachment 11405 [details]
proposed fix

Setting r- due to a couple of issues I see.  

In Base64.cpp:

The * should be next to the type

+inline int strncasecmp(const char *first, const char *second, size_t maxLength)

Though, you should probably use the version of strncasecmp in StringExtras.h if you can.

Please use "unsigned" instead of "unsigned int" in a bunch of places.

Put spaces around infix operators in bunch of places as well.

Also, since this is based on the KDE impl, if there is a bug number or a patch that implemented this it would be nice to have a link to it or at least note who the author of the   patch was.
Comment 4 Alexey Proskuryakov 2006-11-08 09:20:55 PST
Created attachment 11425 [details]
proposed fix

> Though, you should probably use the version of strncasecmp in StringExtras.h if
> you can.

  Done (had to add the header to wtf, however - hope this is OK). Also made this change in DeprecatedString.cpp.

> Please use "unsigned" instead of "unsigned int" in a bunch of places.

  Do we have such a rule? I don't see one in the guidelines, and I don't like this style myself.

> Put spaces around infix operators in bunch of places as well.

  OK, but please note that I intentionally broke this rule in these places - I think adding spaces there has a negative impact on redability. But done.

> Also, since this is based on the KDE impl, if there is a bug number or a patch
> that implemented this it would be nice to have a link to it or at least note
> who the author of the   patch was.

  Base64 implementation comes from kdecore and is completely unrelated to khtml. Its authorship is already reflected in the copyright header.

  The kjs_window part of this implementation is trivial (but note the getOwnPropertySlot question I asked in comment 2). I haven't researched its history.
Comment 5 Alexey Proskuryakov 2006-11-15 10:38:24 PST
(In reply to comment #4)
> (but note the getOwnPropertySlot question I asked in comment 2). 

  OK, I think I see it now. The code is there to allow/deny access to functions from other windows (e.g. window.frames[0].close). Logically speaking, atob() and btoa() are absolutely safe, so there's no reason to disallow them - but Firefox does that, so the patch stands as it is.
Comment 6 Geoffrey Garen 2006-12-07 15:24:09 PST
> > Please use "unsigned" instead of "unsigned int" in a bunch of places.
> 
>   Do we have such a rule? I don't see one in the guidelines, and I don't like
> this style myself.

I'm not sure if it's officially in the guidelines, but Darin harps on it from time to time. I guess being concise is his argument.
Comment 7 Alexey Proskuryakov 2006-12-07 22:00:12 PST
Created attachment 11770 [details]
now with unsigned instead of unsigned int

OK. Would be nice to put this into official guidelines then!
Comment 8 Darin Adler 2006-12-11 15:42:15 PST
Comment on attachment 11770 [details]
now with unsigned instead of unsigned int

Looks good. r=me
Comment 9 Alexey Proskuryakov 2006-12-11 23:05:00 PST
Committed revision 18170.