Bug 28269 - Inspector: Improve Cookie DataGrid to Show Hidden Data
Summary: Inspector: Improve Cookie DataGrid to Show Hidden Data
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-08-13 11:12 PDT by Joseph Pecoraro
Modified: 2009-08-21 19:39 PDT (History)
7 users (show)

See Also:


Attachments
IMAGE: Improved Cookie DataGrid with Hidden Cookie Data (267.77 KB, image/png)
2009-08-13 11:12 PDT, Joseph Pecoraro
no flags Details
IMAGE: Fallback behavior (document.cookie) if platform hasn't implemented raw cookie access (114.07 KB, image/png)
2009-08-13 11:13 PDT, Joseph Pecoraro
no flags Details
Improved Cookies DataGrid (35.41 KB, patch)
2009-08-13 20:35 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
Fixed based on Review (35.53 KB, patch)
2009-08-13 21:48 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
Non-Custom Functions (25.87 KB, patch)
2009-08-19 12:46 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
Fixed Based on Pavel's Comments (25.42 KB, patch)
2009-08-19 13:44 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
Fixed the Cast (25.63 KB, patch)
2009-08-19 14:39 PDT, Joseph Pecoraro
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2009-08-13 11:12:13 PDT
Now that the C++ is in place to access the cookie data, it should be shown in the DataGrid!
Comment 1 Joseph Pecoraro 2009-08-13 11:12:59 PDT
Created attachment 34759 [details]
IMAGE: Improved Cookie DataGrid with Hidden Cookie Data
Comment 2 Joseph Pecoraro 2009-08-13 11:13:42 PDT
Created attachment 34760 [details]
IMAGE: Fallback behavior (document.cookie) if platform hasn't implemented raw cookie access
Comment 3 Joseph Pecoraro 2009-08-13 20:35:06 PDT
Created attachment 34805 [details]
Improved Cookies DataGrid
Comment 4 Joseph Pecoraro 2009-08-13 20:39:34 PDT
C++ NOTES:

  - changed getRawCookies to return a bool, true if implemented, false otherwise
  - Custom Backend Binding will return jsUndefined if unimplemented
  - added deleteCookie, implemented on mac
  - added v8 stubs to prevent a crash

JavaScript NOTES:

  - if getRawCookies was unimplemented resort to fallback behavior
  - fallback behavior just shows cookies Name/Value, no delete button
  - normal behavior shows Name, Value, Domain, Path, Expires, Size, HTTP, Secure
  - delete cookie button is allows for _any_ cookie

QUESTIONS:

  - is "HTTP" the best header name
  - should the UI be updated, for instance the Storage Panel's Sidebar
  - the DataGrid seems crowded when undocked, but looks awesome docked

Cheers
Comment 5 Joseph Pecoraro 2009-08-13 20:43:17 PDT
Just a heads up: I go on vacation early Friday - late Sunday.  Its possible that early Friday I can respond to comments, but otherwise I'll be gone all weekend and unable to respond at all.
Comment 6 Timothy Hatcher 2009-08-13 21:14:58 PDT
Comment on attachment 34805 [details]
Improved Cookies DataGrid


> +    this._useFallback = typeof InspectorController.cookies() === "undefined";

This should be use typeof. We have had this talk before. :)

This does seem expensive for the case when it is implemented, sicne you get the cookie array twice. Maybe you can store the cookies here for use later. Or delate this until the first update is done?
> +            cookies.push(new WebInspector.Cookie(cookie.name, cookie.value, cookie.domain, cookie.path, cookie.expires, cookie.size, cookie.httpOnly, cookie.secure, cookie.session));

We don't need to use WebInspector.Cookie except for the fallback case. You can just directly use the cookie objects in the aray.

> +                cookies.push(new WebInspector.Cookie(name, value, null, null, null, size));

Maybe we can remove WebInspector.Cookie and just make inline objects to fake a cookie. So:

cookies.push({name: name, value: value});

> +        if (String([cookie name]) == cookieName) {

This should be written the other way so you don't convert each cookie's name to C++.

Convert cookieName once to NSString * before the loop. Then do:

if ([[cookie name] isEqualToString:cookieNameString])
Comment 7 Joseph Pecoraro 2009-08-13 21:27:14 PDT
(In reply to comment #6)
> (From update of attachment 34805 [details])
> 
> > +    this._useFallback = typeof InspectorController.cookies() === "undefined";
> 
> This should be use typeof. We have had this talk before. :)

It does use typeof =)

> This does seem expensive for the case when it is implemented, sicne you get the
> cookie array twice. Maybe you can store the cookies here for use later. Or
> delate this until the first update is done?

Since there is no event when cookies change, InspectorController.cookies() is called every time the datagrid is refreshed/updated, this way if some javascript on the page added/removed a cookie, it would be reflected in the next DataGrid update.  If I store it here, when the Panel is created, if any cookies were added via the Console or the Page it wouldn't be reflected in the first draw (when the user first clicks the Cookie Sidebar Section).

> > +            cookies.push(new WebInspector.Cookie(cookie.name, cookie.value, cookie.domain, cookie.path, cookie.expires, cookie.size, cookie.httpOnly, cookie.secure, cookie.session));
> 
> We don't need to use WebInspector.Cookie except for the fallback case. You can
> just directly use the cookie objects in the aray.
> 
> > +                cookies.push(new WebInspector.Cookie(name, value, null, null, null, size));
> 
> Maybe we can remove WebInspector.Cookie and just make inline objects to fake a
> cookie. So:
> 
> cookies.push({name: name, value: value});

Sounds good, I'll remove WebInspector.Cookie.

> > +        if (String([cookie name]) == cookieName) {
> 
> This should be written the other way so you don't convert each cookie's name to
> C++.
> 
> Convert cookieName once to NSString * before the loop. Then do:
> 
> if ([[cookie name] isEqualToString:cookieNameString])

Okay.  I'll take a swing at this!
Comment 8 Joseph Pecoraro 2009-08-13 21:48:53 PDT
Created attachment 34807 [details]
Fixed based on Review

CHANGES:

- Removed WebInspector.Cookie
  - git rm WebCore/inspector/front-end/Cookie.js
  - removed from inspector.html, and WebKit.qrc

- Casted to (NSString *), it looks like PlatformString.h allowed for that with "operator NSString*()"
Comment 9 Joseph Pecoraro 2009-08-13 21:50:58 PDT
(In reply to comment #7)
> > This does seem expensive for the case when it is implemented, sicne you get the
> > cookie array twice. Maybe you can store the cookies here for use later. Or
> > delate this until the first update is done?

Didn't do this. The extra check can be removed once other ports implement it and the fallback behavior is not needed.

> > Maybe we can remove WebInspector.Cookie and just make inline objects to fake a
> > cookie.

Done.

> > Convert cookieName once to NSString * before the loop. Then do:
> > 
> > if ([[cookie name] isEqualToString:cookieNameString])

Done.
Comment 10 Eric Seidel (no email) 2009-08-13 21:56:38 PDT
Comment on attachment 34807 [details]
Fixed based on Review

Rejecting patch 34807 from commit-queue.  This patch will require manual commit.

Patch https://bugs.webkit.org/attachment.cgi?id=34807 from bug 28269 failed to download and apply.
Comment 11 Joseph Pecoraro 2009-08-13 22:04:33 PDT
> This patch will require manual commit.

I was just typing how this would fail due to a git binary diff (localizedStrings)!!  If someone would be kind enough to land this I'd greatly appreciate it.  I'm confident this won't produce a build error.  Otherwise I'll try to get the ball rolling when I get back next week.
Comment 12 Adam Barth 2009-08-15 00:29:10 PDT
I'll try.
Comment 13 Adam Barth 2009-08-15 00:30:58 PDT
I lied.  I'm to go to sleep.  If this patch is still around next time I have some cycles, I'll land it.  Someone else should feel free to land it in the meantime.
Comment 14 Joseph Pecoraro 2009-08-17 08:30:43 PDT
I'm back!  This still applies cleanly to ToT (other then ChangeLogs).  Anyone have the time?
Comment 15 Joseph Pecoraro 2009-08-17 09:05:14 PDT
Landed in r47363 thanks to ariya!
http://trac.webkit.org/changeset/47363
Comment 16 Pavel Feldman 2009-08-17 09:44:05 PDT
Sorry for jumping in late. Few comments from my side:

- I don't see why JSValue JSInspectorBackend::deleteCookie needs to be custom. It can be generated.

- JSValue JSInspectorBackend::cookies can also be implemented in InspectorBackend and only have custom accessors (see InspectorBackend::highlight). Otherwise produces too much duplicated code.

- InspectorController.cookies() should not be called synchronously from the frontend. It is actually not clear how it relates to InjectedScript.getCookies() that is asynchronous. Should latter call into it?
Comment 17 Joseph Pecoraro 2009-08-17 10:06:04 PDT
> - I don't see why JSValue JSInspectorBackend::deleteCookie needs to be custom.
> It can be generated.
> 
> - JSValue JSInspectorBackend::cookies can also be implemented in
> InspectorBackend and only have custom accessors (see
> InspectorBackend::highlight). Otherwise produces too much duplicated code.

This sounds like a good idea.  I'm rather new to this and I don't know the differences between Custom and Non-Custom bindings.  It seems here that a non-Custom is better, and if that is the case then I'm all for it.  Can you point me to some reading, or some code to read, that would help me understand IDL and how WebKit handles bindings?

> - InspectorController.cookies() should not be called synchronously from the
> frontend. It is actually not clear how it relates to
> InjectedScript.getCookies() that is asynchronous. Should latter call into it?

That is a good point.  Right now, InjectedScript.getCookies() is only there for the fallback behavior, by which I mean for platforms that haven't implemented CookieJar's getRawCookies().  You make a good point about making it async.

I can look into renaming the current version as InjectedScript.fallbackGetCookies or something similar and make the current version async with the clearer name.  The end goal is to have all the platforms (or most) implement the new cookies functionality and all of the fallback code can then be removed, but in the meantime they can still get some data (name/value pairs) via the inspected page's document.cookie string.
Comment 18 Pavel Feldman 2009-08-17 10:26:21 PDT
(In reply to comment #17)
> > - I don't see why JSValue JSInspectorBackend::deleteCookie needs to be custom.
> > It can be generated.
> > 
> > - JSValue JSInspectorBackend::cookies can also be implemented in
> > InspectorBackend and only have custom accessors (see
> > InspectorBackend::highlight). Otherwise produces too much duplicated code.
> 
> This sounds like a good idea.  I'm rather new to this and I don't know the
> differences between Custom and Non-Custom bindings.  It seems here that a
> non-Custom is better, and if that is the case then I'm all for it.  Can you
> point me to some reading, or some code to read, that would help me understand
> IDL and how WebKit handles bindings?
> 

You should only use Custom when you need to produce JSC (or V8) specific calls. This includes methods that receive / return DOMObject values in the IDL. In case of your methods you only talk to WebCore and wrap the results in a general way. You should place all code into InspectorBackend, wrap these results into Script(Object|Array|Value) objects and provide small accessors in the cutom classes that would convert JSObject (or v8::Object) into the ScriptObject instances. See InspectorBackend.(un)?wrapObject and how they are being accessed from the custom bindings for reference.

> > - InspectorController.cookies() should not be called synchronously from the
> > frontend. It is actually not clear how it relates to
> > InjectedScript.getCookies() that is asynchronous. Should latter call into it?
> 
> That is a good point.  Right now, InjectedScript.getCookies() is only there for
> the fallback behavior, by which I mean for platforms that haven't implemented
> CookieJar's getRawCookies().  You make a good point about making it async.
> 
> I can look into renaming the current version as
> InjectedScript.fallbackGetCookies or something similar and make the current
> version async with the clearer name.  The end goal is to have all the platforms
> (or most) implement the new cookies functionality and all of the fallback code
> can then be removed, but in the meantime they can still get some data
> (name/value pairs) via the inspected page's document.cookie string.

Actually, things can be even simpler. Here is how I would do this:
- Introduce non-Custom  InspectorBackend::getCookies that would make all necessary computations and call InspectorFrontend::setCookies(String, ScriptObject) that you introduce. Other InspectorFrontend methods for the reference + InspectorResource on how to create a ScriptObject with your data.
- Implement WebInspector.setCookies that would receive and process the data.
- Make a decision on whether to use real cookies or fallback ones right in the InspectorBackend::getCookies().
- Introduce non-Custom InspectorBackend::deleteCookie and do all the same except for it does not need a callback (or does it?).

Anyway, following this pattern you won't need any [Cutom] things at all, you will not need to touch InjectedScript either. InjectedScript is really for the operations that are easier to implement in JavaScript. In your case all the real work is done in C++, so you don't need it.
Comment 19 Joseph Pecoraro 2009-08-18 09:19:29 PDT
> Actually, things can be even simpler. Here is how I would do this:
> - Introduce non-Custom  InspectorBackend::getCookies that would make all
> necessary computations and call InspectorFrontend::setCookies(String,
> ScriptObject) that you introduce. Other InspectorFrontend methods for the
> reference + InspectorResource on how to create a ScriptObject with your data.
> - Implement WebInspector.setCookies that would receive and process the data.

This is certainly a different way of thinking for me.  I'd just rather call a function and get a result.  This approach would mean storing the cookie data somwhere (most likely in the WebInspector namespace), whenever getCookies is called.  I'd rather just call getCookies and get a return value with the data.  What is the advantage to calling a function over just returning a value?
Comment 20 Pavel Feldman 2009-08-18 09:37:07 PDT
(In reply to comment #19)
> > Actually, things can be even simpler. Here is how I would do this:
> > - Introduce non-Custom  InspectorBackend::getCookies that would make all
> > necessary computations and call InspectorFrontend::setCookies(String,
> > ScriptObject) that you introduce. Other InspectorFrontend methods for the
> > reference + InspectorResource on how to create a ScriptObject with your data.
> > - Implement WebInspector.setCookies that would receive and process the data.
> 
> This is certainly a different way of thinking for me.  I'd just rather call a
> function and get a result.  This approach would mean storing the cookie data
> somwhere (most likely in the WebInspector namespace), whenever getCookies is
> called.  I'd rather just call getCookies and get a return value with the data. 
> What is the advantage to calling a function over just returning a value?

Well, you need to pass the result within some callback to make the call asynchronous. If you don't want to store your state / data in between the call and response, look at DOMAgent.js. Here is a snippet you might be interested in:

WebInspector.Cookies.prototype = {
    doAsyncWork: function() {
        function mycallback(results, are, here) {
            // process the result here
        }
        var callId = WebInspector.Callback.wrap(mycallback);
        InspectorController.getCookies(callId, parameters, go, here);
    }
}
WebInspector.didGetCookies = WebInspector.Callback.processCallback;

Then you add 'didGetCookies' into InspectorFrontend and call it from backend with the same callId you are getting.
Comment 21 Joseph Pecoraro 2009-08-18 10:53:16 PDT
> Well, you need to pass the result within some callback to make the call
> asynchronous.

Okay, so the advantage is to make this async.  I wasn't too worried about that, but its probably better to do that now rather then later.

> If you don't want to store your state / data in between the call
> and response, look at DOMAgent.js. Here is a snippet you might be interested
> in:
> 
> WebInspector.Cookies.prototype = {
>     doAsyncWork: function() {
>         function mycallback(results, are, here) {
>             // process the result here
>         }
>         var callId = WebInspector.Callback.wrap(mycallback);
>         InspectorController.getCookies(callId, parameters, go, here);
>     }
> }
> WebInspector.didGetCookies = WebInspector.Callback.processCallback;
> 
> Then you add 'didGetCookies' into InspectorFrontend and call it from backend
> with the same callId you are getting.

Okay, the way that I see this right now, I'll have something like:

  InspectorBackend.getCookies(callId) // Starting point in IDL
   -> InspectorController.getCookies(callid) // To get the Document for cookiesUrl
    -> InspectorDOMAgent.getCookies(callid, doc) // Deal with the CookieJar
     -> InspectorFrontEnd.didGetCookies(callid, data) // Data to send the callback
      -> WebInspector.didGetCookies(callid, data) // Run the JS callback function

It seems like that callstack is a bit over the top but it keeps consistent with current conventions.  Does this look okay?

I'll also need to turn a Vector<Cookie> into a ScriptObject. There are a number of "buildArray..." in InspectorDOMAgent that I could emulate to build an array of cookie objects.  Its actually much easier to work with ScriptObject and ScriptArray then the Custom implementations, I wish I had gone this route before.

Thanks for the guidance so far Pavel, I really appreciate it.
Comment 22 Pavel Feldman 2009-08-18 11:23:00 PDT
Sounds good.

>   InspectorBackend.getCookies(callId) // Starting point in IDL
>    -> InspectorController.getCookies(callid) // To get the Document for
> cookiesUrl

You can skip this one and talk to m_domAgent from within InspectorBackend.cpp. They are friends with InspectorController. It is more of a convention thingy that declares InspectorController's interface to the frontend.
Comment 23 Joseph Pecoraro 2009-08-19 12:46:53 PDT
Created attachment 35133 [details]
Non-Custom Functions

- Removed the [Custom] IDL bindings and instead made simpler Non-Custom bindings that are handled in the Inspector*.cpp files.
- Converted CookiesItemView.js to asynchronously load Cookies via the DOMAgent
Comment 24 Timothy Hatcher 2009-08-19 12:49:00 PDT
Pavel, can you take a look at this patch?
Comment 25 Joseph Pecoraro 2009-08-19 12:50:34 PDT
This should hopefully cover Pavel's comments, but I didn't put it up for review, I'm typing a longer comment (to show up in a minute or so).
Comment 26 Joseph Pecoraro 2009-08-19 13:01:10 PDT
I didn't set that patch as r? because I had some other things that I was
wondering about:

Another developer emailed me with some feedback about the "webkit cookie api". 
He listed a number of good points.  I think I should work on cleaning up and
drafting a good cookie api now, rather then adding it piecewise like I have
been doing.  Some of the suggestions that came out of our email exchange where:

- Change the name of "getRawCookies" to something better like "cookiesList",
and apparently returning a bool is not consistent with other sections.  This
seems more appropriate:

void cookiesList(const Document* document, const KURL& url, Vector<Cookie>&
cookiesList, bool& implemented = false);

- Change the name of "deleteCookie" to "deleteCookieByName" which is more
correct.  I think this is also a good idea.

- It would be nice to prepare for adding support for Creating/Modifying
cookies.  This behavior must already be possible via CookieJar's already
existing setCookies(.., .., String).  I think it would be appropriate to
provide an addCookie convenience function and possibly updateCookieByName.  Or
should these be avoided in favor of specific calls to setCookies?

- He raised a good point as well => "On this topic what is the goal of the
Cookie.h class?  The file didn't contain any comments.  Is it just for the
inspector currently?  Any long term plans?  It would be nice to have a unified
string -> cookie parser for all webkit as it is such as messy job and every
port does it a tiny bit differently."

Any ideas if there could be plans to "unify string cookie parsing" rather then
the current platform specific  mess, or are things too coupled to the network
stacks?
Comment 27 Pavel Feldman 2009-08-19 13:06:16 PDT
This looks so much better from the InspectorController interaction perspective. Couple of comments below:


> +    m_inspectorController->domAgent()->getCookies(callId, m_inspectorController->m_inspectedPage->mainFrame()->document());

domAgent has private mainFrameDocument() method.

> +    value.set("size", (int)(cookie.name.length() + cookie.value.length()));

not sure if WebKit source base allows casts - leaving for Timothy's advice

> +ScriptArray InspectorDOMAgent::buildArrayForCookies(const Vector<Cookie>& cookiesList)

Here and in other places: I try to maintain same order of methods in .h and .cpp


>  
> +void InspectorFrontend::didGetCookies(int callId, const ScriptArray& cookies)
> +void InspectorFrontend::didGetCookies(int callId, const String& cookiesString)

There is no overloading in JavaScript, while InspectorFrontend is supposed to reflect the JavaScript API. I'd either call these differently, or if you are bound to a single name due the callback nature, I would use didGetCookies(int, String, ScriptArray) signature and get rid of extra
boolean flag. In javascript it would be if (cookies) { good code } else  { fallback(cookiesString); }
Comment 28 Joseph Pecoraro 2009-08-19 13:44:45 PDT
Created attachment 35140 [details]
Fixed Based on Pavel's Comments
Comment 29 Joseph Pecoraro 2009-08-19 13:49:58 PDT
(In reply to comment #27)
> This looks so much better from the InspectorController interaction perspective.

Hehe, for me this felt like more work jumping between multiple files with duplication.  But, at least there is no specific bindings, so it works immediately with v8.

> domAgent has private mainFrameDocument() method.

Done. Can't believe I missed that.

> not sure if WebKit source base allows casts - leaving for Timothy's advice

Waiting for comment.

> Here and in other places: I try to maintain same order of methods in .h and
> .cpp

Done. I reordered all my new functions such that if the function comes after X the header, it comes after X in the implementation.

> > +void InspectorFrontend::didGetCookies(int callId, const ScriptArray& cookies)
> > +void InspectorFrontend::didGetCookies(int callId, const String& cookiesString)
> 
> There is no overloading in JavaScript, while InspectorFrontend is supposed to
> reflect the JavaScript API. I'd either call these differently, or if you are
> bound to a single name due the callback nature, I would use didGetCookies(int,
> String, ScriptArray) signature and get rid of extra
> boolean flag. In javascript it would be if (cookies) { good code } else  {
> fallback(cookiesString); }

Done. I did keep the boolean when it actually gets to the CookieItemsView rather then inspecting the Cookie Data itself.
Comment 30 Pavel Feldman 2009-08-19 13:57:11 PDT
Thanks for fixing that.
> 
> Hehe, for me this felt like more work jumping between multiple files with
> duplication.  But, at least there is no specific bindings, so it works
> immediately with v8.
> 

It is more important that we follow the same pattern. InspectorController's interaction will get serialized through the JSON soon, InjectedScript will get into an isolated context. It'll be much easier for me to refactor the code now!
Comment 31 Timothy Hatcher 2009-08-19 13:59:26 PDT
Comment on attachment 35140 [details]
Fixed Based on Pavel's Comments


> +    value.set("size", (int)(cookie.name.length() + cookie.value.length()));

Casting should use C++ casts, like static_cast<int>(cookie.name.length() + cookie.value.length()).

Should this be long?
Comment 32 Joseph Pecoraro 2009-08-19 14:12:39 PDT
(In reply to comment #31)
> (From update of attachment 35140 [details])
> 
> > +    value.set("size", (int)(cookie.name.length() + cookie.value.length()));
> 
> Casting should use C++ casts, like static_cast<int>(cookie.name.length() +
> cookie.value.length()).
> 
> Should this be long?

From this Spec:
http://www.w3.org/Protocols/rfc2109/rfc2109

Implementation Limits:
- at least 4096 bytes per cookie (as measured by the size of the characters that comprise the cookie non-terminal in the syntax description of the Set-Cookie header)

As far as I know most browsers keep this 4k limit.  Developers cannot depend on any more. So an int should be just fine.  The arguments in this case I believe are "unsigned".
Comment 33 Timothy Hatcher 2009-08-19 14:18:49 PDT
Ok, makes sesne. You should use unsigned in your cast unless the set function prevents it.
Comment 34 Joseph Pecoraro 2009-08-19 14:39:35 PDT
Created attachment 35146 [details]
Fixed the Cast

Fixed the cast.

Any comments on Comment 26?
Comment 35 Benjamin Meyer 2009-08-19 20:54:34 PDT
Another thing is that if you compile webkit without inspector these functions (and cookie.h) should probably not be included as they are inspector specific.  Wrapping it in the ifdef would be good.
Comment 36 Joseph Pecoraro 2009-08-21 17:15:03 PDT
(In reply to comment #35)
> Another thing is that if you compile webkit without inspector these functions
> (and cookie.h) should probably not be included as they are inspector specific. 
> Wrapping it in the ifdef would be good.

I don't think there are any generic ifdefs for the inspector. Although maybe there should be if anyone ever wants to build WebKit without the Inspector (small gains?).

Does this patch need anything else?  Any more comments on the API?  Is it worth putting in the work?
Comment 37 Timothy Hatcher 2009-08-21 17:19:48 PDT
Comment on attachment 35146 [details]
Fixed the Cast

Nice clean up.
Comment 38 Joseph Pecoraro 2009-08-21 18:56:14 PDT
- Added the localizedStrings that were supposed to have landed in the first patch but most have gotten lost when ariya did the commit.
- Made "Session" a localizedString as well.  Donno how I missed that before.

Should I close this bug?

---

Landed in r47656 = f0c84dbeb8261b637a75a601fd799530971aa3e5
http://trac.webkit.org/changeset/47656