Summary: | Add support for window.atob() and window.btoa() | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||||||||
Component: | DOM | Assignee: | Alexey Proskuryakov <ap> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Enhancement | ||||||||||||
Priority: | P2 | ||||||||||||
Version: | 420+ | ||||||||||||
Hardware: | Mac | ||||||||||||
OS: | OS X 10.4 | ||||||||||||
URL: | http://developer.mozilla.org/en/docs/DOM:window.atob | ||||||||||||
Attachments: |
|
Description
Alexey Proskuryakov
2006-07-01 03:03:56 PDT
Created attachment 9117 [details]
btoa test
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 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.
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. (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. > > 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.
Created attachment 11770 [details]
now with unsigned instead of unsigned int
OK. Would be nice to put this into official guidelines then!
Comment on attachment 11770 [details]
now with unsigned instead of unsigned int
Looks good. r=me
Committed revision 18170. |