59 bytes, text/x-review-board-request
I noticed that when I'm restoring all my windows on startup, in some of the windows, the web compat item is first in the panel, followed by a separator and then all the built-in items. Also, in new windows, Pocket is now getting placed above the bookmark item, and I'm not sure when that started happening but I don't think it was always broken. At least part of the problem is that BrowserPageActions.init incorrectly calls placeAction with only two arguments instead of three, and the second argument is wrong: https://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser-pageActions.js#51 I'll have a patch soon.
Iteration: --- → 57.2 - Aug 29
Whiteboard: [photon-structure] → [reserve-photon-structure]
Work in progress. Main part's done, working on a test.
I think there are a couple of problems: * BrowserPageActions.init incorrectly calls placeAction with only two arguments instead of three, and the second argument is wrong. * An action added by an extension can get placed in a browser window before that window's BrowserPageActions.init is called -- before that window places all the built-in actions. Which the code does not expect and I think ends up placing the extension's action before all the built-in actions in the panel. I think that's what I'm seeing both when restoring all my windows on startup and with Pocket. I'm not 100% sure, but it's plausible. The fix for the first problem is pretty easy. I added another method to PageActions to get the appropriate place to insert actions in the panel. The second problem points to the fact that PageActions isn't handling the ordering of actions and insertion and removal as robustly as it should be. So most of the changes in this patch address that, and so does the new test. Those changes include: * Stop handling separators specially -- don't assume they should always be appended, don't allow them not to have IDs, etc. * Don't assume there are built-in actions. (Shouldn't happen normally of course, but the test checks this.)
Comment on attachment 8898068 [details] Bug 1391082 - Page action panel ordering can get messed up. https://reviewboard.mozilla.org/r/169370/#review175230 Very nice. Thanks!
Attachment #8898068 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/a08bb8b25b9b Page action panel ordering can get messed up. r=Gijs
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
You need to log in before you can comment on or make changes to this bug.