Closed Bug 868993 Opened 11 years ago Closed 11 years ago

Unresponsive Script warning at CustomizableUI.jsm:436

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 28

People

(Reporter: mconley, Assigned: mconley)

References

Details

Attachments

(2 files)

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.
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)
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
I'm also interested in the currentset attributes for the nav-bar, the TabsToolbar and the PersonalToolbar.
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)
(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)
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)
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).
Ok, thanks everybody, I think I've figured this one out. Patch en route.
Attached patch Patch v1Splinter Review
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
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+
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
Closed: 11 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.

Attachment

General

Creator:
Created:
Updated:
Size: