Closed
Bug 914435
Opened 9 years ago
Closed 9 years ago
some updates from one provider will affect all providers
Categories
(Firefox Graveyard :: SocialAPI, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 27
People
(Reporter: mixedpuppy, Assigned: mixedpuppy)
References
Details
Attachments
(1 file, 2 obsolete files)
11.01 KB,
patch
|
markh
:
review+
|
Details | Diff | Splinter Review |
A bug in one providers worker is setting the user profile every second (bug reported to them). This results in the ui being updated and removing all iframes used in the notification panel, which now may contain iframes from more than one provider. In worse case scenario, you have a panel open at the time this happens. The iframe is removed and the panel becomes a little stub (on osx, probably gone on windows). This happens in SocialToolbar._updateButtonHiddenState where it removes the SharedFrame groups for all iframes.
Assignee | ||
Comment 1•9 years ago
|
||
Assignee: nobody → mixedpuppy
Attachment #804057 -
Flags: review?(mhammond)
Assignee | ||
Comment 2•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=cec3bd224ed9
Comment 3•9 years ago
|
||
Comment on attachment 804057 [details] [diff] [review] fix handling of frames for ambient icons Review of attachment 804057 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/test/social/browser_social_toolbar.js @@ +198,5 @@ > + let numIcons = Object.keys(Social.provider.ambientNotificationIcons).length; > + let ambientIcons = document.querySelectorAll("#social-toolbar-item > toolbarbutton[type='badged']"); > + is(numIcons, ambientIcons.length, "all ambient icons exist"); > + is(panel.childNodes.length, ambientIcons.length + 1, "frames all exist"); > + whitespace here and at line 209 @@ +207,5 @@ > + let icons = document.querySelectorAll("#social-toolbar-item > toolbarbutton[type='badged']"); > + is(icons.length, 0, "ambient icons have been removed"); > + > + // is the status button frame for provider 2 still there? > + for (let f of panel.childNodes) { this block looks like debugging and should either be removed, or the info changed to an ok/is check
Attachment #804057 -
Flags: review?(mhammond) → review+
Assignee | ||
Comment 4•9 years ago
|
||
feeback and new try https://tbpl.mozilla.org/?tree=Try&rev=3b9340db5ad5
Attachment #804057 -
Attachment is obsolete: true
Attachment #804685 -
Flags: review+
Assignee | ||
Comment 5•9 years ago
|
||
see if I get a clean try after some leaky pushes were backed out https://tbpl.mozilla.org/?tree=Try&rev=2fbfa4d9c2e6
Assignee | ||
Comment 6•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/a04d4a573863
Comment 7•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a04d4a573863
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Comment 8•9 years ago
|
||
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/ee7366ad4698 while chasing bug 916757. Feel free to reland after a debug Windows try push with like 30 browser-chrome runs on WinXP and Win8, since we're still not especially sure what got rid of it.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 26 → ---
Assignee | ||
Comment 9•9 years ago
|
||
new winxp/8 try https://tbpl.mozilla.org/?tree=Try&rev=4ad214d5ee9f
Assignee | ||
Comment 10•9 years ago
|
||
winxp/8 try with bug 904104, bug 914926, bug 914927 and bug 914435 https://tbpl.mozilla.org/?tree=Try&rev=6d7e6966f505
Assignee | ||
Comment 11•9 years ago
|
||
this looks like the crash culprit. https://tbpl.mozilla.org/?tree=Try&rev=a1716c65ed3c
Comment 12•9 years ago
|
||
I can't see what could cause this, although it looks like I can repro it locally on about 1 in 10 runs of just the browser/base/content directory. It's probably worth experimenting with more try pushed - eg, avoid creating a worker when remote frameworkers are enabled (which you already suggested in IRC), and I see lots of assertions re https:// in the content process, so maybe even dropping https:// test providers from the patch. I suspect we are going to need help here, so maybe we should continue the conversation in bug 916757 once we have narrowed it down as far as we can.
Assignee | ||
Comment 13•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=49b4c32280ba This avoids the crash in bug 916757 by not using the remote frameworker in the test. Part of the patch (removed) was obsoleted by bug 919803
Attachment #804685 -
Attachment is obsolete: true
Attachment #811342 -
Flags: review?(mhammond)
Updated•9 years ago
|
Attachment #811342 -
Flags: review?(mhammond) → review+
Assignee | ||
Comment 14•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/3fb7c71990d0
Comment 15•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3fb7c71990d0
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
Updated•3 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•