Closed Bug 1437242 Opened 2 years ago Closed 2 years ago

TypeError: statusBroadcaster is null browser-sync.js:126:5 when open Show All Bookmarks

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 60
Tracking Status
firefox-esr52 --- unaffected
firefox58 --- unaffected
firefox59 --- unaffected
firefox60 --- fixed

People

(Reporter: magicp.jp, Assigned: markh)

References

Details

Attachments

(1 file)

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0 ID:20180209220324

Steps to reproduce:
1. Launch Nightly on Windows
2. Open Show All Bookmarks (Ctrl+Shift+B)
   Or...
   Menu bar > Bookmarks > Show All Bookmarks
   Library > Bookmarks > Show All Bookmarks 

Actual results:
The following error is logged in the browser console. I can reproduce on Windows and Ubuntu (without macOS).

TypeError: statusBroadcaster is null  browser-sync.js:126:5


Expected results:
No error


Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=fcd48c850369c0c1845b398a5a09f826603e1547&tochange=e51fa5f76367923d60a62fefdf474ab76fad55c9


Before Bug 767640, the following error are logged in the browser console. See also Bug 1437241.

ReferenceError: Cu is not defined  browser-sync.js:8:1
TypeError: gSync is undefined  places.js:141:5
Blocks: 767640
Has Regression Range: --- → yes
Has STR: --- → yes
Blocks: 406371
See Also: → 1437241
Comment on attachment 8950429 [details]
Bug 1437242 - don't drag browser-sync.js into places.xul.

https://reviewboard.mozilla.org/r/219678/#review225418

I thought I'd make a start on this, but I haven't tested it on iOS and may be missing something...

::: browser/base/content/browser-sync.js:119
(Diff revision 1)
> -    this._generateNodeGetters();
> -    this._definePrefGetters();
> -
>      // initial label for the sync buttons.
>      let statusBroadcaster = document.getElementById("sync-status");
> +    if (!statusBroadcaster) {

I believe the changes here aren't stricly necessary now that places.js doesn't call gSync.init() - but it seems a safe change.

::: browser/components/places/content/places.js
(Diff revision 1)
>      // remove the "Properties" context-menu item, we've our own details pane
>      document.getElementById("placesContext")
>              .removeChild(document.getElementById("placesContext_show:info"));
>  
>      ContentArea.focus();
> -    gSync.init();

And given all our elements are now hidden by default, I don't see why this needs to try and initialize gSync at all?
I suspect this patch will also fix bug 1437241 - even though I can't repro that and don't understand how it could happen, it appears the call stack is via places.js, so avoiding the .init() call from places.js should side-step it.
Assignee: nobody → markh
Thom mentioned in IRC that the intent of his patch was to allow "Sync Now" to remain in that window. This patch removes that, but I think that's fine given the menu only exists on iOS anyway.
Comment on attachment 8950429 [details]
Bug 1437242 - don't drag browser-sync.js into places.xul.

https://reviewboard.mozilla.org/r/219678/#review225774

This removes the 'Sync Now' option from the Tools menu when the places UI is open. Do you think this is worth asking rfeeley about?

::: browser/base/content/browser-sync.js:119
(Diff revision 1)
> -    this._generateNodeGetters();
> -    this._definePrefGetters();
> -
>      // initial label for the sync buttons.
>      let statusBroadcaster = document.getElementById("sync-status");
> +    if (!statusBroadcaster) {

It seems worth being safe here to me.

::: browser/components/places/content/places.js
(Diff revision 1)
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  /* import-globals-from editBookmarkOverlay.js */
>  // Via downloadsViewOverlay.xul -> allDownloadsViewOverlay.xul
>  /* import-globals-from ../../../../toolkit/content/contentAreaUtils.js */
> -/* import-globals-from ../../../base/content/browser-sync.js */

You also should remove it from https://searchfox.org/mozilla-central/source/browser/components/places/content/places.xul#51-52 if we don't need it. Maybe check if there's anything else done in the bug 1384856 patch that needs undoing now that we aren't going to show it in this menu (doesn't look like it).
Attachment #8950429 - Flags: review?(tchiovoloni) → review+
Pushed by mhammond@skippinet.com.au:
https://hg.mozilla.org/integration/autoland/rev/a21c8910699f
don't drag browser-sync.js into places.xul. r=tcsc
https://hg.mozilla.org/mozilla-central/rev/a21c8910699f
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Duplicate of this bug: 1437241
You need to log in before you can comment on or make changes to this bug.