573103 - Implement WebConsole Network Panel

Status: RESOLVED FIXED

(DevTools :: General, defect, P1)

Description

[David Dahl :ddahl]

For each logged entry in the console that is a reference to an object, DOM node, XHR, etc. -  clicking on the entry should open a details panel/widget that shows us what properties it has.

Comment 1

[Julian Viereck]

Note: Bug 573102 is about implementing a PropertyPanel that can be used to inspect JS Objects andDOM nodes. Should this panel be used for XHR data as well (maybe as first iteration and change it later?)

Comment 2

[Julian Viereck]

Just did a short chat with Patrick. His idea was to open up a new panel to display all these information in. That panel then has sections like "Timing", "Request: Header", "Request: Body", "Response: Header", "Response: Body". The look could be somewhat how Joe's last iteration of the advanced CSS Inspector looks like: https://bug582596.bugzilla.mozilla.org/attachment.cgi?id=463090. Keeping the look the same over the dev-tools might be a good idea.

Comment 3

[Julian Viereck]

Attachment: First iteration

This is a first/very early iteration. It's based on Joe's work with some small modifications to the CSS.

The basic idea how the data is presented is taken from WebKit's WebInspector (including the header namings etc). I don't feel good about taking that much over from WebKit, although they have copied Firebug as well.

Please tell me what you think!

Comment 4

[Rob Campbell [:rc] (:robcee)]

Julian, I think that's pretty slick. Very cool and consistent with the new style panel styling Joe's been working on, which I like.

One thing, the twisties in the headers when they're collapsed should be pointing to the right. See the "Timing" header for the example. I'm not sure if that's just because of whatever templates you were using to generate that or if it's the graphic itself.

We might want to adjust the ordering of some of the sections. Timing should probably be last in the list. Request Headers and Response headers should be first and second.

looking good!

Comment 5

[Julian Viereck]

(In reply to comment #4) 
> One thing, the twisties in the headers when they're collapsed should be
> pointing to the right. See the "Timing" header for the example. I'm not sure if
> that's just because of whatever templates you were using to generate that or if
> it's the graphic itself.

No, the twisties are attempted to be like that. If a section is opened, the twisties go down, if a section is closed, they point to the left. But maybe this assumption by me is wrong?

> 
> We might want to adjust the ordering of some of the sections. Timing should
> probably be last in the list. Request Headers and Response headers should be
> first and second.

My idea was to have timing up there as it holds information about all the following "events" (header received etc).

As I'm writting, I'm wondering if that extra section is necessary. Maybe it would be the best to add the timing information to the header information directly, aligned to the right, next beside the twisties? Would save screen space. 

The extra section "Timing" is required if we want to display the timing information using a graph, but that might should be done using a "Networking Timing Panel" that displays all the networking and timing information like it's done in the Firebug Net panel or the Resource section of WebInspector. I expect that designing this graphical display will take more time then we have for this so the simple "put the timing in the header" way could be suitable for now?

---

I wonder how a large chunk of response body should be displayed within the panel. There will be need to add a section "Response Body" that isn't in the markup (yet). If the panel has the current width as in the markup, then a long scrollbar will appear to scroll over the response body. The user can resize the panel to inspect the response body better, but maybe there is an better idea then just resizing the panel.

Also, are there any kind of highlighter in the Firefox codebase already, that could be used to highlight the response body?

Comment 6

[David Dahl :ddahl]

(In reply to comment #5)

> As I'm writting, I'm wondering if that extra section is necessary. Maybe it
> would be the best to add the timing information to the header information
> directly, aligned to the right, next beside the twisties? Would save screen
> space. 

simpler is better right now for sure. see how it looks, but i think we want to err on the side of simple/less code/ file followup bugs

> 
> The extra section "Timing" is required if we want to display the timing
> information using a graph, but that might should be done using a "Networking
> Timing Panel" that displays all the networking and timing information like it's
> done in the Firebug Net panel or the Resource section of WebInspector. I expect
> that designing this graphical display will take more time then we have for this
> so the simple "put the timing in the header" way could be suitable for now?
this is indeed a separate bug (or meta bug)


> I wonder how a large chunk of response body should be displayed within the
> panel. There will be need to add a section "Response Body" that isn't in the
> markup (yet). If the panel has the current width as in the markup, then a long
> scrollbar will appear to scroll over the response body. The user can resize the
> panel to inspect the response body better, but maybe there is an better idea
> then just resizing the panel.
I would justuse the scrollbars for now, perhaps a pop-out window like a view-source window can be used later to allow easier inspection of the response.

> 
> Also, are there any kind of highlighter in the Firefox codebase already, that
> could be used to highlight the response body?
not sure what you are asking for? code highlighter? for markup, yes, css and js maybe not. I'll ask around and look at the view-source code for you.

Comment 7

[Julian Viereck]

Attachment: Second iteration

Compared to the first iteration, this one:

- has no arrows in the headers anymore. This goes with the question: Should the content always be expanded when the panel is opened? Should it be able to collapse the body?

- the "Timing" section is gone. Instead, some simple timing information is added to the headers directly. The number to the right of the "Request Headers" header is the absolute time when the request started + the delta based on when the page is loaded. The number on the "Response Headers" header shows the delta on how long it took from the request till the first response. The time-delta on the "Response Body" shows the time it took to receive the body after the response header was received. During one network request we store up to 12 timing information. The three numbers presented here are based around the question "what are the information a developer will need most". Some developers might care about how long it took to look up the DNS, but I think most are interested in:
  - when did the request start (after I hit "enter" to load/reload the page)
  - how long did it take to get a response from the server (latency)
  - how long did it take to get the response body from the server

Q: Should the "Response Headers|Body" have an absolute time as well?

- added a "Response Body" section with a few lines of content (more then can be displayed in the panel's view port)

Comment 8

[Jeff Balogh (:jbalogh)]

(In reply to comment #7)
> During one network request we store up to 12
> timing information. The three numbers presented here are based around the
> question "what are the information a developer will need most".

DNS and SSL are interesting in the initial requests when those have the biggest impact.  I don't know what the 12 numbers are, but I'd probably want to see them all at some point.  I've filed bugs based on the SSL info from Chrome's debug panel.

It's nice to see the deltas at a glance, but if I'm interested in timing info I'd like to see it all in one place (and with as much detail as you can offer).

Comment 9

[Julian Viereck]

These are the timing infos I record currently - not saying that it might not be possible to add more by using some other Gecko observer mechanism:

      transactionCodes: {
         0x5001: "REQUEST_HEADER",
         0x5002: "REQUEST_BODY_SENT",
         0x5003: "RESPONSE_START",
         0x5004: "RESPONSE_HEADER",
         0x5005: "RESPONSE_COMPLETE",
         0x5006: "TRANSACTION_CLOSE",

         0x804b0003: "STATUS_RESOLVING",
         0x804b0007: "STATUS_CONNECTING_TO",
         0x804b0004: "STATUS_CONNECTED_TO",
         0x804b0005: "STATUS_SENDING_TO",
         0x804b000a: "STATUS_WAITING_FOR",
         0x804b0006: "STATUS_RECEIVING_FROM"
       }

The first attempt of this is to get "something" in. It's not to long until beta5. What do you think, how important is DNS and SSL timing for the majority of developers? Personally, I never was to much interested in.

Comment 10

[Jeff Balogh (:jbalogh)]

I agree that connection setup isn't as generally useful as the pieces you have.  I'll use Chrome if I want to get at the other data for now. ;)

Comment 11

[Julian Viereck]

Attachment: "real" 1

Comment 12

[Julian Viereck]

Attachment: "real 2"

Comment 13

[Julian Viereck]

Uploaded two "real" screenshots that I've taken from the so far implemented NetworkPanel. The screenshot shows the problem when there is more content in one line then it can be displayed in it (so x-scrollbars appear). Also, a very large response body doesn't look that nice + shows up scroll bars (see "real 2").

Comment 14

[Julian Viereck]

Comment on attachment 464843 [details]
"real 2"

Please note the comments/ideas/thoughts in the bug so far as well.

Comment 15

[Kevin Dangoor]

That's generally looking good!

Can there be multiple of these open at a time? If so, the title bar should probably reflect either the time of the request or the URL requested.

Comment 16

[Julian Viereck]

Attachment: "real" 3

Added new section "Sent Cookies". This data is sent with the request header and as such could be in the same section but I have the feeling this is something independent enough to open up a separate section.

To prevent the text leaking to the right of the sections I've added overflow-x: auto in the css. This causes scrollers to appear within the sections. To prevent the Response Body section to be giant I've added a max-height=350px to the sections.

UX question: Are there now to many scrollers? How will this behave on non OSX platforms?

This version looks good enough to me so I will stick with it as long as that's okay for the UX experts. Note that I might add arrows back to expand, close a header by clicking on the section.

Comment 17

[Julian Viereck]

Attachment: "real" 4 (Linux screenshot)

This is a screenshot of the current NetworkPanel made on Gnome (clearlooks theme) Ubuntu 10.04.

Comment 18

[Julian Viereck]

Attachment: Patch v1

Patch that implements the NetworkPanel. I sent this and another patch to try server. If there are no leakings, then I will start the feedback/review process.

Comment 19

[Julian Viereck]

Attachment: Patch v2

Updated patch. This patch depends on the attachments in bug 568634. It includes a working NetworkPanel that works for inspecting text + (cached) images.

Comment 20

[Julian Viereck]

Requesting blocking status on this bug as it's an important feature for the WebConsole. It allows the user to inspect single network requests.

Comment 21

[Julian Viereck]

Attachment: patch v3

Added some more comments, added more i10n support, ensure that the unit tests are running alone and not in parallel to other tests (caused orange in try build) + other smaller improvements.

Comment 22

[Julian Viereck]

Attachment: patch v3.1

Rebased patch + improved l10n support + small cleanup.

Comment 23

[David Dahl :ddahl]

Comment on attachment 467129 [details] [diff] [review]
patch v3.1

the only thing I don't like is the 162 line method: "update: function NP_update()"

Perhaps you can re-factor that into several methods? does that make sense? if not, why not. It will be harder to maintain.

Looking good though. Will build it tomorrow.

Comment 24

[Julian Viereck]

Attachment: Patch v4

New NetworkPanel that has a much simplier update() function. Thanks for pointing this out David.

Comment 25

[David Dahl :ddahl]

This patch is badly bitrotted, can you rebase. I just need to run tests and one last look.

Comment 26

[Kevin Dangoor]

The patch bitrotted in 22 minutes?

Comment 27

[Julian Viereck]

(In reply to comment #25)
> This patch is badly bitrotted, can you rebase. I just need to run tests and one
> last look.

Sorry, David, I should have told you what you need to apply this:

- patch from bug 588345 (Fix l10n comments for JS/DOM Object Introspector)
- patch from bug 587573 (Log image requests to the WebConsole)

I shouldn't have based that patches on top of bug 588345 as this is not related to this work but it already has an r+ just waiting for blocking/approval which means it's nearly about to get checked in and I expect it to land before this patches will do.

Tell me if that's okay for testing purpose, otherwise I upload a rebased patch that is not build on top of bug 588345 (which would require some work... that's why I'm asking).

Comment 28

[Julian Viereck]

As there is some conversation going on about displaying network request duration in the WebConsole (bug 568634) I raise my hand for feedback on the l10n parts of this bug as well. The new added l10n section looks currently like this:

  NetworkPanel.label=Inspect Network Request
  NetworkPanel.requestHeaders.label=Request Headers
  NetworkPanel.requestCookie.label=Sent Cookie
  NetworkPanel.requestBody.label=Request Body
  NetworkPanel.responseHeaders.label=Response Headers
  NetworkPanel.responseBody.label=Response Body
  NetworkPanel.responseNoBody.label=There Is No Response Body
  NetworkPanel.responseImage.label=Received Image
  NetworkPanel.responseImageCached.label=Cached Image
  # LOCALIZATION NOTE (NetworkPanel.deltaDurationMS):
  #
  # This string is used to show the duration between two network events (e.g
  # request and respones header or response header and response body).
  #
  # `Δ` results in the greek capital delta character.
  # For more information see the page:
  #   http://www.fileformat.info/info/unicode/char/394/index.htm
  NetworkPanel.deltaDurationMS=Δ%Sms
  # LOCALIZATION NOTE (NetworkPanel.imageSizeDeltaDurationMS):
  # This string is used to show the duration between the response header and the
  # response body event. It also shows the size of the received or cached image.
  #
  # The first %S is replace by the width of the inspected image.
  # The second %S is replaced by the height of the inspected image.
  # The third %S is replaced by the duration between the response header and the
  # response body event.
  #
  # `Δ` results in the greek capital delta character.
  # For more information see the page:
  #   http://www.fileformat.info/info/unicode/char/394/index.htm
  NetworkPanel.imageSizeDeltaDurationMS=%Sx%Spx, Δ%Sms

I don't want to explain anything because it should be already there - if not, please let me know and I add more information as needed.

Comment 29

[David Dahl :ddahl]

(In reply to comment #27)
> Sorry, David, I should have told you what you need to apply this:
> 
> - patch from bug 588345 (Fix l10n comments for JS/DOM Object Introspector)
> - patch from bug 587573 (Log image requests to the WebConsole)
> 
> I shouldn't have based that patches on top of bug 588345 as this is not 

Aha. well, i think we should really get bug 587573 at least fully reviewed before we spend too much time reviewing this patch as there will be a bunch of rebasing to do. I will land bug 587573 ASAP as it is IMHO int the top 2 most important patches.

Comment 30

[David Dahl :ddahl]

Comment on attachment 467433 [details] [diff] [review]
Patch v4

Looking good, will f+ soon, maybe after 587573 is reviewed:)

Comment 31

[Dietrich Ayala (:dietrich)]

Sounds like a great enhancement, but not holding the release for it, blocking-.

Comment 32

[Julian Viereck]

Attachment: Patch v5

This patch replaces is changes a lot: The NetworkPanel is no longer a html file but instead an XHTML file to have support for DTDs. A new DTD is added that holds the i10n strings for the basic UI (this was suggested by Axel). Also, this patch makes sure the NetworkPanel's document is loaded before it's accessed.

Comment 33

[Julian Viereck]

Attachment: Patch v5.1

Sorry David for the spam... One string got lost during the merge.

Comment 34

[Julian Viereck]

Attachment: Patch v5.2

Rebased patch.

Comment 35

[David Dahl :ddahl]

Comment on attachment 468674 [details] [diff] [review]
Patch v5.2

I like the functionality, but I think after speaking with Kevin and Limi, it might be worth our time to see what happens when we either dock this output in a vbox and insert it to the right of the console output, OR, display it inline in the console the way that firebug does it, basically appending this into an output node. You would want to collapse all of it, with an obvious way to expand the output. I'm just not sure the panels are the way to go.

Comment 36

[Julian Viereck]

Attachment: Integration1

(In reply to comment #35)
> Comment on attachment 468674 [details] [diff] [review]
> Patch v5.2
> 
> I like the functionality, but I think after speaking with Kevin and Limi, it
> might be worth our time to see what happens when we either dock this output in
> a vbox and insert it to the right of the console output

A *VERY* simple/fast hack to put the NetworkPanel to the right. Shows an collapsed "Request Headers".

Comment 37

[Julian Viereck]

Attachment: Integrate2

Same idea as Integration1 (attachment 468783 [details]), showing a problem when the requested URL is very long...

Comment 38

[Julian Viereck]

Attachment: Integrate1.1

Comment 39

[Julian Viereck]

Attachment: Integration3

This screenshot shows an UX problem: if there is to much content in the response body. The height of the response body div is limited to a fixed number using "min-height" in the css of the XHTML. If the height of the WebConsole is smaller then this min-height, then the section header "Response Body" is no longer visible and as such the user doesn't know what he is inspecting. Also, see the entire content of the response body scrolling in the response body div as well as in the panel itself is necessary which just feels wrong.

A solution for this might be to set the min-height value depending on the WebConsole height. If some just could tell me how to do this... Can we modify CSS rules using some internal XPCOM magic?

Comment 40

[Alex Limi (:limi) — Firefox UX Team]

Right — I wouldn't even try to show the response object in that space, can we link to a dedicated window for it instead? It's (usually) going to be of equivalent size and content of an HTML page, isn't it? If it is, we should give it a real window, like with View Source.

If there are two classes of response objects, can we display the smaller ones inline and link off to a dedicated window if the size exceeds some threshold?

Comment 41

[David Dahl :ddahl]

limi: I just started a thread around these ideas: 

http://groups.google.com/group/mozilla-developer-tools/browse_thread/thread/b58c2a03edc0fca2

Comment 42

[Kevin Dangoor]

Looking at the amount of work to do and the resources available, I just don't think it's possible to rework the UI for these now. I think they need to stay as panels.

Comment 43

[David Dahl :ddahl]

Comment on attachment 468674 [details] [diff] [review]
Patch v5.2

Since we want to get this landed, we should just do it as a panel to get the string in, and file and inline view followup bug

Comment 44

[Julian Viereck]

Attachment: Patch v5.3

Small improvements + rebased.

Comment 45

[Kevin Dangoor]

This patch is the basic "view of HTTP requests" for the console and, as such, it is both an important feature *and* has strings, so this should really be a beta 5 or 6 blocker.

This patch has been looked at within our group but has not yet gone through review, so landing it in beta 5 may be unrealistic. Since the patch is already here, though, I propose we land early in beta 6.

Comment 46

[Kevin Dangoor]

Julian just pointed out that this bug is blocking other bugs (which are already listed as beta5+ blockers). I'll see what the chances are of getting this reviewed today.

Comment 47

[Dietrich Ayala (:dietrich)]

Comment on attachment 469154 [details] [diff] [review]
Patch v5.3


>+///////////////////////////////////////////////////////////////////////////
>+//// Helper for creating the panel.

nit: s/panel/network panel/

>+/**
>+ * Creates a new DOMNode and appends it to aParent.
>+ *
>+ * @param nsIDOMDocument aDocument
>+ *        Document to create the new DOMNode.
>+ * @param nsIDOMNode aParent
>+ *        A parent node to append the created element.
>+ * @param string aTag
>+ *        Name of the tag for the DOMNode.
>+ * @param object aAttributes
>+ *        Attributes set on the created DOMNode.
>+ *
>+ * @returns nsIDOMNode
>+ */
>+function appendChild(aDocument, aParent, aTag, aAttributes)
>+{
>+  let node = createElement(aDocument, aTag, aAttributes);
>+  aParent.appendChild(node);
>+  return node;
>+}

createAndAppendElement would be more descriptive. also, why not just make parent an optional arg to createElement?

>+
>+///////////////////////////////////////////////////////////////////////////
>+//// NetworkPanel
>+
>+/**
>+ * Creates a new NetworkPanel.
>+ *
>+ * @param nsIDOMNode aParent
>+ *        Parent node to append the created panel to.
>+ * @param nsIDOMDocument aDocument
>+ *        Document to create the new nodes on.

could leave this out, using aParent.ownerDocument instead.

>+  // Create the tree.

nit: s/tree/something else/

>+  // Destroy the panel when it's closed.
>+  this.panel.addEventListener("popuphidden", function onPopypHide() {
>+    self.panel.removeEventListener("popuphidden", onPopypHide, false);

typo

>+  /**
>+   * Callback is called once the NetworkPanel is ready and updated once. Used
>+   * by unit tests.
>+   */
>+  isReadyCallback: null,

hm, can't you listen for the event sent when a popup is loaded, and check that popup's id for a match?

>+  /**
>+   *
>+   * @returns boolean
>+   *          Returns true if the server responded that the request is already
>+   *          in the browser's cached, false otherwise.

typo: cached.

>+  /**
>+   * Appends the node with id=aId by the text aValue.
>+   *
>+   * @param string aId
>+   * @param string aValue
>+   * @returns void
>+   */
>+  _appendNode: function NP_appendNode(aId, aValue)
>+  {
>+    let textNode = this.document.createTextNode(aValue);
>+    this.document.getElementById(aId).appendChild(textNode);
>+  },

nit: _appendTextNode for clarity

>+
>+  /**
>+   * Adds a list of key-values to the node with id=aPartenId.

typo

also, this comment doesn't describe what actually happens. add a note explaining that it builds actual HTML content for display?

ftr, i think the styling needs work, but ui-review will weigh in on that. biggest issue to me is that the value data is not aligned against center vertical line, making the general appearance of the panel jumbled, and making it difficult for the eye to quickly scan the contents.

>+  /**
>+   * Processes the request header.

what do you mean by process, and what's it do with the processed content? ditto for the processing methods following this.

>+      let cookies = request.header.Cookie.split(";");
>+      let cookieList = {};
>+      let cookieListSorted = {};
>+      cookies.forEach(function(cookie) {
>+        let name, value;
>+        [name, value] = cookie.trim().replace("\n", "").split("=");

can this also be \r\n or \r? can't remember for cookie content...

>   unregisterDisplay: function HS_unregisterDisplay(aId)
>   {
>+    // Remove children from the output. If the output is not cleared, there can
>+    // be leaks as some nodes has node.onclick = function; set and GC can't
>+    // remove the nodes then.
>+    HUDService.clearDisplay(aId);
>+

why are you doing that then? why not use addEventListener and remove the listener when appropriate?

>+<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en">
>+<head>
>+  <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"/>
>+  <style>

split this out into a separate file, move to the theme dirs.


>+  </div>
>+</div>
>+</body>
>+</html>
>\ No newline at end of file

fix

>+<!ENTITY networkPanel.requestHeaders              "Request Headers">
>+<!ENTITY networkPanel.requestCookie               "Sent Cookie">
>+<!ENTITY networkPanel.requestBody                 "Request Body">
>+<!ENTITY networkPanel.responseHeaders             "Response Headers">
>+<!ENTITY networkPanel.responseBody                "Response Body">
>+<!ENTITY networkPanel.responseNoBody              "There Is No Response Body">

s/There Is// IMO, but ask Limi.

r=me for what's here so far. however, i'd like to see another revision of this post-UI-review, and with the alignment issue i mentioned above fixed.

i'm also concerned about the platform agnostic look and feel. but i guess issues with that will shake out in ui-review.

Comment 48

[Julian Viereck]

Attachment: Patch v5.4

Same as Patch v5.3 but replaced "Contenet" by "Content".

Comment 49

[Julian Viereck]

Attachment: Patch v6

Dietrich, thanks a lot for jumping in and doing the review :)

This patch addresses most of your review comments. Some notes:


> >+  /**
> >+   * Processes the request header.
> 
> what do you mean by process, and what's it do with the processed content? ditto
> for the processing methods following this.

> >+
> >+  /**
> >+   * Adds a list of key-values to the node with id=aPartenId.
> 
> 
> also, this comment doesn't describe what actually happens. add a note
> explaining that it builds actual HTML content for display?
 
I've added some more comments that explain what happen in each function. The _process* functions are now renamed to _display*.

> ftr, i think the styling needs work, but ui-review will weigh in on that.
> biggest issue to me is that the value data is not aligned against center
> vertical line, making the general appearance of the panel jumbled, and making
> it difficult for the eye to quickly scan the contents.

Will discuss this with Limi. I can see your point but I think as the key values are bold/black and the values are slightly grayed there is enough difference to keep the keys and values apart.

> >+      let cookies = request.header.Cookie.split(";");
> >+      let cookieList = {};
> >+      let cookieListSorted = {};
> >+      cookies.forEach(function(cookie) {
> >+        let name, value;
> >+        [name, value] = cookie.trim().replace("\n", "").split("=");
> 
> can this also be \r\n or \r? can't remember for cookie content...

Question: Does it need the "\n" here at all? The cookies are sent within the header which is separated by \r|\n|\r\n so there can't be any kind of linebreak in the header cookie string.

> >   unregisterDisplay: function HS_unregisterDisplay(aId)
> >   {
> >+    // Remove children from the output. If the output is not cleared, there can
> >+    // be leaks as some nodes has node.onclick = function; set and GC can't
> >+    // remove the nodes then.
> >+    HUDService.clearDisplay(aId);
> >+
> 
> why are you doing that then? why not use addEventListener and remove the
> listener when appropriate?

Well... you're right. However this is much easier to implement then keeping track of each node with an onclick event to clear it up later. That would be much more work and if we should do it, I would be glad to put that in a followup bug so that we don't block this bug for much longer.

Comment 50

[Julian Viereck]

Attachment: Patch v6.1

Rebasing + added missing CSS file for gnome theme.

Comment 51

[Dietrich Ayala (:dietrich)]

> Will discuss this with Limi. I can see your point but I think as the key values
> are bold/black and the values are slightly grayed there is enough difference to
> keep the keys and values apart.

i will defer to judgement of the UX review! if you haven't already, ping Limi and ask if he's going to be able to review this immediately. if not, ask him what other UX person can review it immediately :)

> Question: Does it need the "\n" here at all? The cookies are sent within the
> header which is separated by \r|\n|\r\n so there can't be any kind of linebreak
> in the header cookie string.

good question, i don't know. look it up?

> Well... you're right. However this is much easier to implement then keeping
> track of each node with an onclick event to clear it up later. That would be
> much more work and if we should do it, I would be glad to put that in a
> followup bug so that we don't block this bug for much longer.

ok, we can live w/ it.

Comment 52

[Dietrich Ayala (:dietrich)]

Comment on attachment 469913 [details] [diff] [review]
Patch v6.1


>   /**
>    * Assign a function to this property to listen for finsihed httpRequests.
>    * Used by unit tests.

typo. still not happy with this test-only code in here. file a followup to fix it later?

r=me otherwise.

Comment 53

[Alex Limi (:limi) — Firefox UX Team]

Comment on attachment 464843 [details]
"real 2"

I think this works fine for now, I'm still working on enhancements to the style panel that can probably be applied to this panel to improve whitespace & overall layout during the polish phase. I believe we have all the functionality we need.

Comment 54

[Shawn Wilsher :sdwilsh]

(In reply to comment #52)
> typo. still not happy with this test-only code in here. file a followup to fix
> it later?
fwiw, I allowed it in a different bug too.  We should strive to not have it though whenever possible.

Comment 55

[Julian Viereck]

(In reply to comment #54)
> (In reply to comment #52)
> > typo. still not happy with this test-only code in here. file a followup to fix
> > it later?
> fwiw, I allowed it in a different bug too.  We should strive to not have it
> though whenever possible.

Right, it's ugly. Before I stored the last request directly on the HUDService object but that caused some leaking. That's why I've introduced this callback. Will open a followup bug.

Comment 56

[Julian Viereck]

Attachment: Patch v6.2

Rebased patch.

Comment 57

[Julian Viereck]

Attachment: [backed-out] Patch v6.3

Rebased patch + modified to work with new patch in bug 568634.

Comment 58

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 470109 [details] [diff] [review]
[backed-out] Patch v6.3

http://hg.mozilla.org/mozilla-central/rev/fa93ab4adc7a

Comment 59

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 470109 [details] [diff] [review]
[backed-out] Patch v6.3

http://hg.mozilla.org/mozilla-central/rev/289e6da90526

leaks

Comment 60

[David Dahl :ddahl]

http://hg.mozilla.org/mozilla-central/rev/e39d03333cdf

after tryserver success, one more push