929826 - Improve about:memory's UI for the multi-process case

Status: RESOLVED FIXED

(Toolkit :: about:memory, defect)

Description

[Nicholas Nethercote [inactive]]

Now that we have multi-process for real, in about:memory it can be hard to tell which process's data you are looking at.  In particular, my standard operation of hitting PgDn to see the summary stats for the main process is no longer valid, because it might be a content process at the bottom.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Improve about:memory's UI for the multi-process case.

This patch makes two changes to the UI.

- Each process' section now gets an <h3> at the bottom that says "End of
  <process name>".

- There are now links between the top and bottom of a process' section.

I'll attach a screenshot shortly.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: screenshot

The down/up arrows take you to the end/start of a process' measurements.

Comment 3

[John Schoenick [:johns]]

Comment on attachment 820783 [details] [diff] [review]
Improve about:memory's UI for the multi-process case.

Review of attachment 820783 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine, minor nit would be that the underlined ↓ looks like a "collapse" action, maybe |text-decoration: none| it or add small "next process" text

::: toolkit/components/aboutmemory/content/aboutMemory.js
@@ +1331,5 @@
> +    let link = appendElementWithText(aP, "a", "noselect", aArrow);
> +    link.href = "about:memory#" + aThere + aN;
> +    link.id = aHere + aN;
> +    link.title = "Go to the " + aThere + " of " + aProcess;
> +    appendElementWithText(aP, "span", "", "\n");

nit - Literal newlines and other whitespace are collapsed outside of <pre> tags, you should use a <br> or just append the link to the <h3> tag with a "h3>a { not so giant }" style.

Comment 4

[Nicholas Nethercote [inactive]]

> This looks fine, minor nit would be that the underlined ↓ looks like a
> "collapse" action, maybe |text-decoration: none| it or add small "next
> process" text

I tried adding text and it looked horrible, so I went with |text-decoration: none|.


> nit - Literal newlines and other whitespace are collapsed outside of <pre>
> tags

...except when you copy and paste into a text buffer, where the newlines affect how it looks :)  aboutMemory.js is littered with newlines like these so that the pasted text looks good.

---

One thing I don't like about this patch is that the location updates to something like about:memory#end0 when you click on an arrow.  And then if you scroll back up to the top and re-hit "measure", the #end0 is ignored and just sits there looking ugly.  Is there any way to avoid this?

Comment 5

[John Schoenick [:johns]]

(In reply to Nicholas Nethercote [:njn] from comment #4)
> > This looks fine, minor nit would be that the underlined ↓ looks like a
> > "collapse" action, maybe |text-decoration: none| it or add small "next
> > process" text
> 
> I tried adding text and it looked horrible, so I went with |text-decoration:
> none|.
> 
> 
> > nit - Literal newlines and other whitespace are collapsed outside of <pre>
> > tags
> 
> ...except when you copy and paste into a text buffer, where the newlines
> affect how it looks :)  aboutMemory.js is littered with newlines like these
> so that the pasted text looks good.

Ah, right, carry on then!

> One thing I don't like about this patch is that the location updates to
> something like about:memory#end0 when you click on an arrow.  And then if
> you scroll back up to the top and re-hit "measure", the #end0 is ignored and
> just sits there looking ugly.  Is there any way to avoid this?

You can handle the scrolling yourself and ignore anchors:

> mylink.addEventListener("click", function(event) {
>   // The target.href would be "#end0" which can double as a selector
>   document.documentElement.scrollTop = document.querySelector(event.target.href).offsetTop;
>   event.preventDefault();
> }, false);

This assumes you're using href="#anchor" instead of "about:memory#anchor" so it can just pass the href to querySelector. (and that link.offsetParent == document.documentElement)

Alternatively, If you want to honor the #end0 after re-measuring, just re-set the hash when the page changes

>   if (location.hash) {
>     location.hash = location.hash;
>   }

Comment 6

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c79088ab4e0c

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c79088ab4e0c