Added the InfoToolTip to Cookie Panel #33

Merged
merged 7 commits into from Aug 24, 2012

Projects

None yet

3 participants

@bharaththiruveedula

No description provided.

@SebastianZ
Member

According to my comment at issue 5834 the infotip should contain info about the raw and decoded cookie size.

Also please note that when you're fixing an issue that is listed inside our issue tracker, you should refer to it by giving the commit a comment like this:

Issue 5834 (Add infotip for cookie size)
http://code.google.com/p/fbug/issues/detail?id=5834

Sebastian

@janodvarko
Member

Good, just wanted to do the review (I promised that to Bharath yesterday when helping him to implement that)

Couple of additional comments:

  • cookie.sizeinfo.Cookie_Size string needs to be created in cookies.properties file
  • The cookie object (in cookie.js module) should have getSize() method as we discussed yesterday.

Honza

@SebastianZ
Member
  • cookie.sizeinfo.Cookie_Size string needs to be created in cookies.properties file

Also the key should be renamed to cookies.sizeinfo.Size because "Size" is enough.

Honza, there is already a cookies.header.size in cookies.properties. Should that be renamed to e.g. cookies.size or cookies.data.size instead, so it can be used for both, the header and the infotip?

Sebastian

@janodvarko
Member

Honza, there is already a cookies.header.size in cookies.properties. Should that be renamed to e.g.
cookies.size or cookies.data.size instead, so it can be used for both, the header and the infotip?
No, let's have two different string keys. They are using in different contexts and it could happen
that we want to slightly modify the label in one of them.

Honza

@janodvarko
Member
  1. Use yet cookie name to generate the infoTipCookieId (something like "cookiesize-" + cookie.name)
  2. this.infoTipFile should be this.infoTipCookie
  3. the descTag is still in the code, it should be removed
  4. Sebastian pointed that sizeInfo needs to include the raw size of the cookie as well as the decoded size (but just when it differs from the raw size). I don't see this.

Honza

@SebastianZ
Member

Also the translation should just be "Size", not "Cookie Size". And for the decoded value size it should be "Decoded Size".

Sebastian

@janodvarko
Member
  1. Bharath, you need to pay more attention to the changes. If you change this.infoTipFile to this.infoTipCookie, you need to change all occurrences (see line 494)

  2. Please do not create lines longer than 100 characters
    sizeInfo.push({label: Locale.$STR("cookie.sizeinfo.Size"), size: size},{label: Locale.$STR("cookie.sizeinfo.RawSize"), size: rawSize});
    see: https://getfirebug.com/wiki/index.php/Firebug_Coding_Style

  3. And for the decoded value size it should be "Decoded Size".
    Since the data tab says "Raw Data" I think the size label should use "Raw Size"

  4. As Sebastian mentioned the getCookie should look like:

getSize: function()
{
    return this.cookie.name.length + this.cookie.value.length;
},

Bharath, is the difference clear?

The same for getRawSize()

Honza

@janodvarko janodvarko merged commit 243d583 into firebug:master Aug 24, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment