(Resolve) Error due to Backbone.History being started again

[tfrommen]

When using Backbone, an error is thrown if you (try to) start the history multiple times. For regular browsers, this means that JavaScript is effectively not working at all.

In WordPress core, the history is just started without further ado. And this leads to problems when you have a plugin (or a theme, for that matter) that uses Backbone itself (and that maybe even checks for an already started history). Depending on the dependencies of individual JavaScript files, the plugin JS might be included earlier than the core JS. The plugin checks for a running history, doesn't find one, starts one, ... and then core comes along and implicitly triggers an error.

This could be resolved by checking the history status before starting it, and then maybe either don't do anything at all, or stop (and then restart) the history.
In my opinion, core is allowed to configure Backbone as needed, so stopping an already started history and then (re)starting it like core wants it to be is totally fine in my opinion.

So, why isn't there a check right now?
Does anyone see any problem when integrating such checks?

Any third-party JS code is, of course, required to configure the root of the Backbone history just like core does - otherwise everything will break anyway.
And if some plugin author decides to ignore this (or maybe just doesn't know this!), this would lead in either the core JS or the plugin JS not working properly (routing wouldn't happen as expected). But it does not result in no JavaScript at all being functional, which is a big plus in my opinion. :)

So, once again, instead of doing this:

Backbone.history.start({ /* options */ });

we could either do this:

if ( ! Backbone.History.started ) {
    Backbone.history.start({ /* options */ });
}

or that:

if ( Backbone.History.started ) {
    Backbone.history.stop();
}
Backbone.history.start({ /* options */ });

Any opinions?

[adamsilverstein]

@tfrommen Thanks for the bug report.

This seems like a sensible change. Would you like to work on a patch?

[tfrommen]

Replying to adamsilverstein:

@tfrommen Thanks for the bug report.

This seems like a sensible change. Would you like to work on a patch?

Sure, will provide a patch soon.

What version do you prefer? I think stopping and (re)starting exactly like core it needs is better than only starting if not yet started.

[adamsilverstein]

What version do you prefer? I think stopping and (re)starting exactly like core it needs is better than only starting if not yet started.

I'm not sure.

Can you provide some sample code or do you know of a plugin that does this? I'd like to test how history behaves in the 2 scenarios. my first instinct is the first version: only start in not started already - assuming history still works as expected.

[tfrommen]

Replying to adamsilverstein:

What version do you prefer? I think stopping and (re)starting exactly like core it needs is better than only starting if not yet started.

I'm not sure.

Can you provide some sample code or do you know of a plugin that does this? I'd like to test how history behaves in the 2 scenarios. my first instinct is the first version: only start in not started already - assuming history still works as expected.

Well, when starting the history, you can only define four things that are native: root, hashChange, pushState and silent. The first one, root, will default to /, and should be consistent between core, plugins and themes, anyway. The other three options are Booleans, and depending on their individual value, the behavior of the history might be different.

Another thing is, that you can define custom options that will live on the history, for example, to be used by other parties. In this case, it is no problem, though, if core were to stop and then restart the history, because the options that will be set are not exactly what you pass to start(). Instead, you will only overwrite the values of the options you define yourself. In other words, if some plugins defines a custom option, and core stops and restarts the history, that third-party option is not lost. Which is good, of course.

I would suggest that core stops and restarts the history, so this is what you can find in my first patch.

Maybe someone could slurp the plugins repository to specifically check for Backbone.history.start, or so, but I wouldn't expect lots of exotic stuff in that regard.

[SergeyBiryukov]

Replying to tfrommen:

Maybe someone could slurp the plugins repository to specifically check for Backbone.history.start, or so, but I wouldn't expect lots of exotic stuff in that regard.

Searched the repo, found 50 matches in 43 files:

an-gradebook/js/app/router/GradeBookRouter.js
buddydrive/includes/js/buddydrive-app.js
buddydrive/includes/js/buddydrive-app.min.js
cf-whiteboard/js/all-athletes.js
clinked-client-portal/app/clinked-portal.js
collaborate-notes/admin/js/collaborate-notes-admin.js
custom-contact-forms/assets/build/js/form-manager.js
custom-contact-forms/assets/js/manager/app.js
custom-contact-forms/build/js/form-manager.js
easing-slider/assets/js/admin.js
easing-slider/assets/js/admin.min.js
easing-slider/resources/assets/js/admin.js
easy-appointments/js/admin-router.js
easy-appointments/js/admin.prod.js
et-mailing/inc/ae/assets/js/option-view.js
frames-video-gallery/admin/scripts/script-0.1.5.js
gust/assets/helpers.js
icegram/assets/js/gallery.min.js
krux-apps/views/admin_page.php
krux-smb/views/admin_page.php
mainwp/js/mainwp-theme.js
mobilechief-mobile-site-creator/lib/fontawesome/docs/assets/js/index/index.js
multilingual-press/assets/js/admin.js
multilingual-press/assets/js/admin.min.js
picu/frontend/js/picu-app.js
plugins-enabler/js/app.js
pocha-slider/js/editor-pages.js
pocha-slider/js/editor-pages.min.js
posts-timeline/post-timeline/poststimeline.php
qoob/qoob/js/qoob.js
qoob/qoob/qoob.min.js
rainbowpaypress/RainbowPayPress.php
sendpress/js/spnl-backbone.js
simple-history/js/scripts.js
street-view-comments/js/views.js
supercharts/scripts/js/wizard.min.js
thx38/thx-38.js
woocommerce-predictive-search/assets/js/predictive-search-results.backbone.min.js
wp-backbone/static/js/app.run.js
wp-email-delivery/assets/js/admin.js
wp-email-delivery/assets/js/admin.min.js
wp-ultimate-search/js/main-pro.js
wp-ultimate-search/js/wpus-main.js

[tfrommen]

Replying to SergeyBiryukov:

Replying to tfrommen:

Maybe someone could slurp the plugins repository to specifically check for Backbone.history.start, or so, but I wouldn't expect lots of exotic stuff in that regard.

Searched the repo, found 50 matches in 43 files:

an-gradebook/js/app/router/GradeBookRouter.js
buddydrive/includes/js/buddydrive-app.js
buddydrive/includes/js/buddydrive-app.min.js
cf-whiteboard/js/all-athletes.js
clinked-client-portal/app/clinked-portal.js
collaborate-notes/admin/js/collaborate-notes-admin.js
custom-contact-forms/assets/build/js/form-manager.js
custom-contact-forms/assets/js/manager/app.js
custom-contact-forms/build/js/form-manager.js
easing-slider/assets/js/admin.js
easing-slider/assets/js/admin.min.js
easing-slider/resources/assets/js/admin.js
easy-appointments/js/admin-router.js
easy-appointments/js/admin.prod.js
et-mailing/inc/ae/assets/js/option-view.js
frames-video-gallery/admin/scripts/script-0.1.5.js
gust/assets/helpers.js
icegram/assets/js/gallery.min.js
krux-apps/views/admin_page.php
krux-smb/views/admin_page.php
mainwp/js/mainwp-theme.js
mobilechief-mobile-site-creator/lib/fontawesome/docs/assets/js/index/index.js
multilingual-press/assets/js/admin.js
multilingual-press/assets/js/admin.min.js
picu/frontend/js/picu-app.js
plugins-enabler/js/app.js
pocha-slider/js/editor-pages.js
pocha-slider/js/editor-pages.min.js
posts-timeline/post-timeline/poststimeline.php
qoob/qoob/js/qoob.js
qoob/qoob/qoob.min.js
rainbowpaypress/RainbowPayPress.php
sendpress/js/spnl-backbone.js
simple-history/js/scripts.js
street-view-comments/js/views.js
supercharts/scripts/js/wizard.min.js
thx38/thx-38.js
woocommerce-predictive-search/assets/js/predictive-search-results.backbone.min.js
wp-backbone/static/js/app.run.js
wp-email-delivery/assets/js/admin.js
wp-email-delivery/assets/js/admin.min.js
wp-ultimate-search/js/main-pro.js
wp-ultimate-search/js/wpus-main.js

Wow, thanks a lot, @SergeyBiryukov! I will have a look at what these plugins do exactly.

[tfrommen]

So, here are the details on how the above plugins start the history...

---

an-gradebook/js/app/router/GradeBookRouter.js:

Backbone.history.start();

buddydrive/includes/js/buddydrive-app.js:

Backbone.history.start();

cf-whiteboard/js/all-athletes.js:

Backbone.history.start({
    root: CFW_OPTIONS.athletes_page_path,
    pushState: ! 0
});

clinked-client-portal/app/clinked-portal.js:

Backbone.history.start({
    silent: true
});

This plugin also ships Backbone itself - which is far from ideal, of course.

collaborate-notes/admin/js/collaborate-notes-admin.js:

Backbone.history.start();

custom-contact-forms/assets/build/js/form-manager.js:

Backbone.history.start();

custom-contact-forms/assets/js/manager/app.js:

Backbone.history.start();

custom-contact-forms/build/js/form-manager.js:

Backbone.history.start();

easing-slider/resources/assets/js/admin.js:

Backbone.history.start({
    root: window._easingsliderAdminL10n.admin_url,
    pushState: true
});

easy-appointments/js/admin-router.js:

Backbone.history.start();

et-mailing/inc/ae/assets/js/option-view.js:

Backbone.history.start();

frames-video-gallery/admin/scripts/script-0.1.5.js:

Backbone.history.start();

gust/assets/helpers.js:

Backbone.history.start({
    root: '/ghost',
    hashChange: false,
    pushState: true
});

icegram/assets/js/gallery.min.js:

I was unable to find any reference to Backbone.history in trunk.

krux-apps/views/admin_page.php:

Backbone.history.start();

krux-smb/views/admin_page.php:

This plugin's ​SVN repository is empty?!

mainwp/js/mainwp-theme.js:

Backbone.history.start({
    root: themes.data.settings.adminUrl,
    hashChange: false,
    pushState: true
});

mobilechief-mobile-site-creator/lib/fontawesome/docs/assets/js/index/index.js:

Backbone.history.start({
    pushState: false
});

multilingual-press/assets/js/admin.js:

Backbone.history.start({
    root: this.settings.urlRoot,
    hashChange: false,
    pushState: true
});

MultilingualPress does check if the history has been started already, but this is not enough if MultilingualPress runs before WordPress core - which is exactly the reason why I created this ticket in the first place. This is the ​GitHub issue.

picu/frontend/js/picu-app.js:

Backbone.history.start({
    pushState: false
});

plugins-enabler/js/app.js:

Backbone.history.start();

pocha-slider/js/editor-pages.js:

Backbone.history.start({
    root: window._wpMediaGridSettings.adminUrl,
    pushState: true
});

posts-timeline/post-timeline/poststimeline.php:

Backbone.history.start();

qoob/qoob/js/qoob.js:

Backbone.history.start({
    pushState: false
});

rainbowpaypress/RainbowPayPress.php:

backbone.history.start();

sendpress/js/spnl-backbone.js:

Backbone.history.start();

simple-history/js/scripts.js:

Backbone.history.start();

street-view-comments/js/views.js:

Backbone.history.start();

supercharts/scripts/js/wizard.min.js:

Backbone.history.start();

thx38/thx-38.js:

Backbone.history.start({
    pushState: true,
    silent: true
});

woocommerce-predictive-search/assets/js/predictive-search-results.backbone.min.js:

Backbone.history.start({
    root: wc_ps_results_vars.search_page_path,
    pushState: ! 0
});

wp-backbone/static/js/app.run.js:

Backbone.history.start();

wp-email-delivery/assets/js/admin.js:

Backbone.history.start();

wp-ultimate-search/js/main-pro.js:

Backbone.history.start();

wp-ultimate-search/js/wpus-main.js:

Backbone.history.start();

---

A few observations:

• there are no custom options defined;
• all plugins that define a root different from WordPress core, are (hopefully) not active on the admin pages where core is (e.g., Media), otherwise there should've been reports about either the plugin or core JavaScript not being functional;
• similar to before, all plugins using silent: true shouldn not be active on the pages where core is;
• the option pushState: false can be ignored, because this is the default anyway (Backbone sets the internal option to !!options.pushState, so there is no difference between having undefined and explicitly saying false);
• almost none of the above plugins performs a check for an already started history itself.

The last item is important, in my opinion, because it shows that there shouldn't ba any problem at all with having core stop and restart the history. The reason is simple: if the plugins's JavaScript was active on one or more of the pages where core is active, either the individual plugin or core would throw an Error - which would lead to some issues reported.

So, I can only repeat: let's make stop the history (if started, of course), and restart it just like core requires it to be. In other words: apply the suggested patch. :)

Any opinions on this?

[adamsilverstein]

@tfrommen thanks for taking the time to research the other uses in plugins!

I agree that if we had existing conflicts we would have heard about them. Stopping/starting history when already started as you do in 39612.patch​ seems reasonable as an approach.

[adamsilverstein]

In 40076:

JavaScript: when starting Backbone history, stop if previously started.

Prevent a potential error condition if Backbone history is started by a plugin or theme before core tries to start it.

Props tfrommen.
Fixes #39612.