Closed
Bug 685929
Opened 13 years ago
Closed 12 years ago
Multiple widgets position in same toolbar not persistent after restart
Categories
(Add-on SDK Graveyard :: General, defect, P2)
Add-on SDK Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
1.6
People
(Reporter: r3gis, Assigned: ochameau)
Details
Attachments
(1 file)
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:9.0a1) Gecko/20110909 Firefox/9.0a1 Build ID: 20110909032331 Steps to reproduce: * Create a widget that add more than one widget * Customize widget position and put all in the navigation bar for example (important point is that it must be the same toolbar) * Restart firefox Actual results: Only the first widget is now located on the navigation bar. All other widget are in addon bar. Expected results: All widgets should be in the navigation bar as before restart.
Reporter | ||
Comment 1•13 years ago
|
||
Possible addons to reproduce issue : https://addons.mozilla.org/fr/firefox/addon/googlemusic-remote-control/ OR https://addons.mozilla.org/fr/firefox/addon/grooveshark-remote-control/ But can be reproduced with a simple basic addon that create two widgets. My first analysis is that problem is in widget.js in function _insertNodeInToolbar and introduced by the two last lines : ------- // Update DOM in order to save position if we remove/readd the widget container.setAttribute("currentset", container.currentSet); // Save DOM attribute in order to save position on new window opened this.window.document.persist(container.id, "currentset"); ------- In fact what happen is that when there is more than one widget saved in currentSet for an addon, this methods will at the first run store container.currentSet. Problem is that at this point, container.currentSet does not contains yet the other widgets of the addon. So it store the list of widgets *without* other widgets of the addon (not yet added). As consequence next runs of the method will not detect that the widget should go in the customized toolbar. Commenting these two lines seems to solve the problem and to not introduce regression for use case defined as per comment and commit message that introduce these two lines. But I'm not sure of this last point, so should be analyzed more deeply :). Besides, regarding my analysis, I think that it could also apply to widgets of all addons not yet added to the bar. And that one is able to customize position only of one jetpack widget in a toolbar.
Comment 2•13 years ago
|
||
Hmm, this definitely sounds like undesired behavior! :-) Alex, Hernán: can y'all confirm this problem?
Whiteboard: [triage:followup]
Comment 3•13 years ago
|
||
(In reply to Myk Melez [:myk] from comment #2) > Hmm, this definitely sounds like undesired behavior! :-) > > Alex, Hernán: can y'all confirm this problem? Uhm, I've seen this come up in the grooveshark addon issue tracker, but been too busy to have a look at it. I would vote for confirmed though.
Comment 4•13 years ago
|
||
Confirmed! (Feel free to do this yourself for bugs you think warrant it!)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 5•13 years ago
|
||
(In reply to Myk Melez [:myk] from comment #4) > Confirmed! (Feel free to do this yourself for bugs you think warrant it!) Yup, I warrant it and confirm it now :) I tested with latest source and GRC 0.3.2 and can reproduce on Firefox 6.0.2/Ubuntu 11.04
Priority: -- → P2
Whiteboard: [triage:followup]
Comment 6•12 years ago
|
||
We got one report from our MemChaser users who also can see this bug regularly, because he moved the widget into the navigation bar.
Comment 7•12 years ago
|
||
Any updates on this?
Assignee | ||
Comment 8•12 years ago
|
||
Sorry guys for the delay, I should have looked at this way before! So the patch from bug 656298 allowed to ensure that widget are displayed in the expected toolbar/offset when we open a new window. That's because Firefox-and-Widget.js codes are using this `currentset` DOM attribute in order to restore toolbaritems positions. But this code ended up conflicting with itself, when multiple addons were running this code on Firefox startup, or when one addon used multiple widgets in the same toolbar. The code is now even more complex and not 100% satisfying, but I think that the only way to implement the whole thing sanely would be to handle widgets in Firefox, or in a unique instance of a Widget module. In the meantime, this bug is being reported in almost every major Jetpack addons, like Collusion, MemChaser, ... We have read many angry users comments about "Widget disappearing on each startup". I think and hope it was solely because of this bug. It sounds really important to get it fixed ASAP. dietrich, feel free to forward this review to irakli if you do not have time to look at it!
Assignee: nobody → poirot.alex
Attachment #609719 -
Flags: review?(dietrich)
Assignee | ||
Comment 9•12 years ago
|
||
Hernán, Henrik, would you have some time to give a try to this patch?
Assignee | ||
Comment 10•12 years ago
|
||
Comment on attachment 609719 [details]
Pull request 388
Trying to get this in 1.6, so we will try to get it landed quickly.
Attachment #609719 -
Flags: review?(dietrich) → review?(rFobic)
Updated•12 years ago
|
Attachment #609719 -
Flags: review?(rFobic) → review+
Comment 11•12 years ago
|
||
This is working! I tested it in Firefox 11 and Aurora on OSX 10.6.
Comment 12•12 years ago
|
||
Commits pushed to master at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/1430b712bc2c7f11309eb87788fefd154e0c4d46 Bug 685929 - Multiple widgets position in same toolbar not persistent after restart https://github.com/mozilla/addon-sdk/commit/778701a9400151df2115238d8c2d64500e5467be Merge pull request #388 from ochameau/bug/685929-fix-widget-positions fix Bug 685929 - Multiple widgets position in same toolbar not persistent after restart r=@gozala
Updated•12 years ago
|
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•12 years ago
|
Status: RESOLVED → UNCONFIRMED
Ever confirmed: false
Resolution: FIXED → ---
Target Milestone: --- → 1.6
Assignee | ||
Updated•12 years ago
|
Status: UNCONFIRMED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Comment 13•12 years ago
|
||
Commit pushed to stabilization at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/de78ccb1c23ea6a78675fc572151ef27d9eca58e Bug 685929 - Multiple widgets position in same toolbar not persistent after restart (cherry-picked from commit 1430b712bc2c7f11309eb87788fefd154e0c4d46)
You need to log in
before you can comment on or make changes to this bug.
Description
•