All users were logged out of Bugzilla on October 13th, 2018

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

RESOLVED FIXED in mozilla27

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

unspecified
mozilla27
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Created attachment 820783 [details] [diff] [review]
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.
Attachment #820783 - Flags: review?(jschoenick)
(Assignee)

Comment 2

5 years ago
Created attachment 820786 [details]
screenshot

The down/up arrows take you to the end/start of a process' measurements.
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.
Attachment #820783 - Flags: review?(jschoenick) → review+
(Assignee)

Comment 4

5 years ago
> 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?
Flags: needinfo?(jschoenick)
(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;
>   }
Flags: needinfo?(jschoenick)
Blocks: 879538

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/c79088ab4e0c
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.