light blue glow in about:memory app tab

RESOLVED FIXED in mozilla13

Status

()

Toolkit
about:memory
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Stefan, Assigned: njn)

Tracking

Trunk
mozilla13
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Created attachment 599644 [details]
blue glow.jpg

User Agent:  

Steps to reproduce:

1. Pin about:memory as an app tab.
2. Restart Fx.


Actual results:

2. The app tab is colored light blue. Decolors if clicked at.


Expected results:

2. No color.
(Reporter)

Comment 1

6 years ago
First affected nightly: 2012-02-22-03-12-23-mozilla-central
OS: Other → Linux
Hardware: Other → x86_64

Comment 2

6 years ago
I think that is intentional. The app tabs turn on this highlight behind the site icon when anything changes in the tab. Try something like gmail or other webmail or just some refreshing webpage.

One the other hand about:memory should not change after first render AFAIK.
(Reporter)

Comment 3

6 years ago
(In reply to :aceman from comment #2)
> I think that is intentional. 

Is there a bug id?

> The app tabs turn on this highlight behind the
> site icon when anything changes in the tab. Try something like gmail or
> other webmail or just some refreshing webpage.

I do have an application running in an app tab which---whenever the content has changed---does not turn on the highlight.

> One the other hand about:memory should not change after first render AFAIK.

Anyway.

Comment 4

6 years ago
I see the glow on my webmail tab, when it refreshes and updates the count of new messages. I assume it must be an intentional feature. It could be bug 577096.

This could also be a dupe of bug 605111.
(Reporter)

Comment 5

6 years ago
(In reply to :aceman from comment #4)
> It could be bug 577096.

Yes.  Thanks for the reference.  There must be a specific change in the title in order to trigger this.

Comment 6

6 years ago
I think any change in title suffices. Maybe the loading of the tab was slow and FF noticed the change from "New tab" to "about:memory" and made the highlight. If you see it only in recent nightlies, maybe the developers are currently playing with some new handling of new tabs.
(Reporter)

Comment 7

6 years ago
(In reply to :aceman from comment #6)
> I think any change in title suffices. 

   document.title += ' '

is not sufficient, 

   var tmp = document.title;
   document.title = ''
   document.title = tmp;

is not either.
(Assignee)

Comment 8

6 years ago
about:memory's title used to be set via a <title> tag in the .xhtml file.  Since bug 702300, which implemented about:compartments, landed, the title is now set via JS.  See http://mxr.mozilla.org/mozilla-central/source/toolkit/components/aboutmemory/content/aboutMemory.js#115.  I guess that's the cause.

Both about:memory and about:compartments are implemented by aboutMemory.{xhtml,js}.  I guess I could create aboutCompartments.xhtml that is identical to aboutMemory.xhtml except for the title.
(Assignee)

Comment 9

6 years ago
Created attachment 600837 [details] [diff] [review]
patch

This patches changes the parsing of location.href.

- It's now done before the onload handler, which is early enough that
  there's no blue glow on start-up if about:memory is an app tab.

- It's now case-insensitive, which means that addresses like ABOUT:MEMORY
  and About:Compartments?Verbose now work;  previously they caused
  exceptions to be thrown.  (Thanks to mccr8 for noticing this.)  But the
  document.title is still always either "about:memory" or
  "about:compartments".

I updated the tests to test the latter change, but not the former change
because it's obscure and hard to test.
Assignee: nobody → n.nethercote
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #600837 - Flags: review?(jschoenick)

Updated

6 years ago
Assignee: n.nethercote → nobody
Component: Untriaged → about:memory
Product: Firefox → Toolkit
QA Contact: untriaged → about.memory

Updated

6 years ago
Assignee: nobody → n.nethercote
OS: Linux → All
Hardware: x86_64 → All
Comment on attachment 600837 [details] [diff] [review]
patch

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

This looks good. Comment above about the regex part, but r=me either way.

::: toolkit/components/aboutmemory/content/aboutMemory.js
@@ +57,5 @@
>  const UNITS_PERCENTAGE       = Ci.nsIMemoryReporter.UNITS_PERCENTAGE;
>  
>  // Because about:memory and about:compartments are non-standard URLs,
>  // location.search is undefined, so we have to use location.href here.
> +// The regexps are case insensitive so that typing something like

The long else-if with regex's here seems unnecessarily verbose, I would do something like:

> let gVerbose;
> {
>   let split = document.location.href.split('?');
>   document.title = split[0].toLowerCase();
>   gVerbose = split.length == 2 && split[1].toLowerCase() == 'verbose';
> }
Attachment #600837 - Flags: review?(jschoenick) → review+
(Assignee)

Comment 11

6 years ago
> > let gVerbose;
> > {
> >   let split = document.location.href.split('?');
> >   document.title = split[0].toLowerCase();
> >   gVerbose = split.length == 2 && split[1].toLowerCase() == 'verbose';
> > }

That's much better, thanks.

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