345260 - handle dynamic changes to tab title, tab icon and busy (loading) state in all tabs menu

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect, P2)

Description

[(not reading, please use [e-mail] instead)]

handle dynamic changes to uri, image/favicon, title in all tabs menu

this can happen if changes occur after the menupopup is built.

from tabbrowser.xml

+            // XXX todo
+            // what if uri, image/favicon, title change 
+            // while this popup is open?
+            // mano warns:  "be careful of leaks when addressing this."
+            menuItem.setAttribute("label", curTab.label);
+            menuItem.setAttribute("image", curTab.getAttribute("image"));
+            var URI = curTab.linkedBrowser.currentURI.spec;

Comment 1

[(not reading, please use [e-mail] instead)]

I don't think this is blocker worthy for 2.0.  

it would be nice to have, but I think we could ship without fixing this.

what do others think?

Comment 2

[Peter Kasting]

(In reply to comment #1)
> I don't think this is blocker worthy for 2.0.  

I'm inclined to say it's a blocker.  If the URI/title/favicon change and the menu shows you the old one, not only is that totally useless, but it's actively showing you *wrong* information that it clearly has correct elsewhere (in the actual tab itself).

I'd probably agree if it were just dynamic favicon changes, but the other two feel more serious to me.

I would certainly classify this below the "handle dynamic tab addition/removal properly" bug though, if only one could get fixed.

Comment 3

[Asaf Romano (gone)]

Taking

Comment 4

[Asaf Romano (gone)]

We don't need to keep the uri (which is stored in the statusbartext attribute only) in sync since the statusbar stuff isn't implemented yet.

Comment 5

[Asaf Romano (gone)]

Attachment: patch

This:
 1. Adds support for showing the busy (loading) indicator in the menu.
 2. Makes sure we crop the menuitem label in the same way we crop the tab label (i.e. we crop it in the middle if the tab label is the page URL).
 3. Keeps the attributes of the menuitems (label, image, crop, busy) in sync with their corresponding tabs.

Comment 6

[(not reading, please use [e-mail] instead)]

asaf, how much work would it be to keep the status text in sync, too?

Comment 7

[Bryan]

(In reply to comment #6)
> asaf, how much work would it be to keep the status text in sync, too?

Please, let's not go back to unsync'd status bars "messages".

~B

Comment 8

[Asaf Romano (gone)]

(In reply to comment #6)
> asaf, how much work would it be to keep the status text in sync, too?
> 

Not much, but I don't want to do this before you're implementing the statusbar stuff (Really, if we're not going to use the menubar code as is, we don't need this attribute).

Comment 9

[(not reading, please use [e-mail] instead)]

Comment on attachment 230217 [details] [diff] [review]
patch

r=sspitzer, sorry for the delay.

Comment 10

[Asaf Romano (gone)]

trunk:
mozilla/toolkit/content/widgets/tabbrowser.xml 1.174
mozilla/toolkit/themes/winstripe/global/browser.css 1.16
mozilla/toolkit/themes/pinstripe/global/browser.css 1.12

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 230217 [details] [diff] [review]
patch

a=drivers. Please land on the MOZILLA_1_8_BRANCH.

Comment 12

[Asaf Romano (gone)]

1.8 branch:
mozilla/toolkit/content/widgets/tabbrowser.xml 1.103.2.62
mozilla/toolkit/themes/pinstripe/global/browser.css 1.7.4.5
mozilla/toolkit/themes/winstripe/global/browser.css 1.9.4.9