Unresponsive Script warning at CustomizableUI.jsm:436

RESOLVED FIXED in Firefox 28

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mconley, Assigned: mconley)

Tracking

Trunk
Firefox 28
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
I've had a few reports of this bug since last Friday. Not entirely sure what's going on here, but I'm going to check it out.
(Assignee)

Comment 1

5 years ago
I'm most interested in the value of browser.uiCustomization.state for these users.

Here's one:

{"placements":{"PanelUI-contents":["new-window-button","zoom-controls","print-button","history-button","fullscreen-button","share-page-5","bookmarks-panelmenu","home-button"],"nav-bar":["unified-back-forward-button","urlbar-container","bookmarks-menu-button-container","reload-button","stop-button","openinchrome-btn","search-container","downloads-button","webrtc-status-button"]},"seen":[]}
Flags: needinfo?(sm.1975.smith)

Comment 2

5 years ago
Mine is:

{"placements":{"PanelUI-contents":["new-window-button","print-button","history-button","fullscreen-button","history-panelmenu","bookmarks-panelmenu"],"TabsToolbar":["tabbrowser-tabs","new-tab-button","alltabs-button","tabs-closebutton"],"PersonalToolbar":["personal-bookmarks"],"nav-bar":["search-container","bookmarks-menu-button-container","bookmarks-menu-button","downloads-button","social-toolbar-button","PanelUI-button","share-page"],"toolbar-menubar":["menubar-items","forecastfox-menubar-spring"]},"seen":[]}

UA: Mozilla/5.0 (X11; Linux x86_64; rv:23.0) Gecko/20130504 Firefox/23.0
(Assignee)

Comment 3

5 years ago
I'm also interested in the currentset attributes for the nav-bar, the TabsToolbar and the PersonalToolbar.

Comment 4

5 years ago
Hi Mike,

This was in the prefs.js file I archived when trying to sort the problem out:

user_pref("browser.uiCustomization.state", "{\"placements\":{\"PanelUI-contents\":[\"print-button\",\"new-window-button\",\"feed-button\",\"fullscreen-button\",\"restartlessrestart-toolbarbutton\",\"bookmarks-panelmenu\"],\"nav-bar\":[\"search-container\",\"menu-button\",\"bookmarks-menu-button\",\"downloads-button\",\"bookmarks-button\",\"tabview-button\",\"social-toolbar-button\",\"PanelUI-button\"]},\"seen\":[]}");

Luckily, I knew this might be coming, so I archived the profile before I ran the updated build.  If you need me to, I can replace my current profile with the archived contents and give it a whirl again.  I am remembering the unresponsive script warning as what you have used as the title of this report.
Flags: needinfo?(sm.1975.smith)
(Assignee)

Comment 5

5 years ago
(In reply to Sean Smith from comment #4)
> user_pref("browser.uiCustomization.state",
> "{\"placements\":{\"PanelUI-contents\":[\"print-button\",\"new-window-
> button\",\"feed-button\",\"fullscreen-button\",\"restartlessrestart-
> toolbarbutton\",\"bookmarks-panelmenu\"],\"nav-bar\":[\"search-container\",
> \"menu-button\",\"bookmarks-menu-button\",\"downloads-button\",\"bookmarks-
> button\",\"tabview-button\",\"social-toolbar-button\",\"PanelUI-button\"]},
> \"seen\":[]}");
> 
> Luckily, I knew this might be coming, so I archived the profile before I ran
> the updated build.

Ah, excellent. Thanks for thinking ahead. :)

The other data I need is the currentset attributes for your nav-bar, your TabsToolbar, your PersonalToolbar and your toolbar-menubar.

You can find that data inside of your localstore.rdf file for that profile. Just search around for the string "currentset", and you should be able to pick them out.
Flags: needinfo?(sm.1975.smith)

Comment 6

5 years ago
Here's the current value of the preference you were requesting, after having sorted the problem out:

user_pref("browser.uiCustomization.state", "{\"placements\":{\"PanelUI-contents\":[\"new-window-button\",\"print-button\",\"history-button\",\"fullscreen-button\",\"feed-button\",\"sync-button\",\"greasemonkey-tbb\",\"home-button\",\"restartlessrestart-toolbarbutton\"],\"TabsToolbar\":[\"tabbrowser-tabs\",\"new-tab-button\",\"alltabs-button\",\"tabs-closebutton\"],\"PersonalToolbar\":[\"personal-bookmarks\"],\"nav-bar\":[\"unified-back-forward-button\",\"urlbar-container\",\"reload-button\",\"stop-button\",\"search-container\",\"webrtc-status-button\",\"bookmarks-menu-button-container\",\"menu-button\",\"bookmarks-menu-button\",\"downloads-button\",\"social-toolbar-button\",\"tabview-button\"],\"toolbar-menubar\":[\"menubar-items\"]},\"seen\":[]}");

I looked around for "currentset" in the archived localstore.rdf and in my current localstore.rdf and didn't find that search term.  Rather than wasting your time trying to explain to me what you need, I will attach the archived localstore.rdf file for you.
Flags: needinfo?(sm.1975.smith)

Comment 7

5 years ago
Created attachment 745862 [details]
Archived localstore.rdf file - pre-startup of updated UX build

Comment 8

5 years ago
One additional test I did... I restored the archived profile, disabled all my extensions, and on restart, still get the unresponsive script warning.  If I hit Stop, the window opens with the goofy toolbar, but I CAN move items around.  On close and restart, I still get the script warning, and the toolbar is again jumbled (but things are movable).
(Assignee)

Comment 9

5 years ago
Ok, thanks everybody, I think I've figured this one out. Patch en route.
(Assignee)

Comment 10

5 years ago
Created attachment 745882 [details] [diff] [review]
Patch v1

Here's what happened:

When a toolbar registers itself with CustomizableUI.jsm, any state for that toolbar is loaded from prefs, and then we "build the toolbar" - meaning that we take out the default items that aren't supposed to be there anymore, and put in the things that the user wants, all in the right order.

We do that in two phases - first, we go through the items that the user has in their state, and inject them into the toolbar from the beginning, while skipping over the items that are "in the right place".

The end result is a toolbar with the right things at the start, and a possibly non-empty set of items at the end of the toolbar that need to be removed.

We set the limit to be the node just after the last user-wanted item, and then work backwards from the end of the toolbar, removing items and tossing them into the palette if they're removable. If they're not removable, we leave them be.

The problem was that after we'd chucked an item X into the palette, we went to the next iteration by choosing that chucked item's previous sibling Y. *But the item X had already moved into the palette*, so now the previous sibling Y is an item that's also in the palette. This means that the item Y is certainly not the limit, so the while-loop continues. And if the item is removable (and of course it is, since Y was in the palette!), then we'd chuck Y at the end of the palette. And then choose *it's* previous sibling - which was item X again!

And boom, infinite loop.

The solution is to make sure we're keeping a reference to the items previous sibling *before* we move it into the palette.
Assignee: nobody → mconley
Status: NEW → ASSIGNED
(Assignee)

Updated

5 years ago
Attachment #745882 - Flags: review?(jaws)
Comment on attachment 745882 [details] [diff] [review]
Patch v1

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

r=me. Nice investigation :)
Attachment #745882 - Flags: review?(jaws) → review+
(Assignee)

Comment 12

5 years ago
Thanks for the review!

Jamun: https://hg.mozilla.org/projects/jamun/rev/2689867ed1fa
UX: https://hg.mozilla.org/projects/ux/rev/2689867ed1fa
Whiteboard: [fixed-in-jamun][fixed-in-ux]
https://hg.mozilla.org/mozilla-central/rev/2689867ed1fa
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-jamun][fixed-in-ux]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.