Add support for updating the document hash, off by default, when the browser history is updated (issue 5753)

[Snuffleupagus]

This is really the best that we can do here, since other proposed solutions would interfere with (and break) the painstakingly implemented browsing history that's present in the default viewer.

I'm still not convinced that this is a good idea in general, but this patch implements it in a way where it is possible to toggle[1] for users that wish to have this feature. In particular, there's a couple of reasons why I'm not finding this feature necessary/great:

• It's already possible to easily obtain the current hash, by simply clicking on the viewBookmark button at any time.
• Hash changes requires a bit of special handling[2], i.e. extra code, to prevent issues when the browser history is traversed (see PDFHistory._popState). Currently this is only necessary when the user has manually changed the hash, with this patch it will always be the case (assuming the feature is active).
• It's not always possible to change the URL when updating the browser history. For example: In the Firefox built-in viewer, the URL cannot be modified for local files (i.e. those using the file:// protocol).
  This leads to inconsistent behaviour, and may in some cases even result in errors being thrown and the history thus not updating, if the browser prevents changes to the URL during pushState/replaceState calls.

---

[1] Using the historyUpdateUrl viewer preference.

[2] This depends, to a great extent, on browsers always firing popstate events before hashchange events, which may or may not actually be guaranteed.

[timvandermeij]

/botio-linux preview

[pdfjsbot]

From: Bot.io (Linux m4)

---

Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/4fa1744e7fb619d/output.txt

[pdfjsbot]

From: Bot.io (Linux m4)

---

Success

Full output at http://54.67.70.0:8877/4fa1744e7fb619d/output.txt

Total script time: 1.66 mins

Published

• Viewer: http://54.67.70.0:8877/4fa1744e7fb619d/web/viewer.html

[timvandermeij]

/botio-linux preview

[pdfjsbot]

From: Bot.io (Linux m4)

---

Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/85175e4d3c56967/output.txt

[pdfjsbot]

From: Bot.io (Linux m4)

---

Success

Full output at http://54.67.70.0:8877/85175e4d3c56967/output.txt

Total script time: 1.64 mins

Published

• Viewer: http://54.67.70.0:8877/85175e4d3c56967/web/viewer.html

[timvandermeij]

I'm fine with this since it's off by default. Thank you!

[KiaraGrouwstra]

That looks great, thank you @Snuffleupagus! 🎉

[Rob--W]

This feature has no "title" key in preferences_schema.json. This prevents the option from being rendered in the settings page of the Chrome extension.

Preferences should only be hidden if they are internal, non-user facing or instable work-in-progress features. It seems that this feature does not match that description.

@Snuffleupagus If this feature is ready for use by users, could you open a PR and add a meaningful title to the preferences_schema.json, to expose the pref to users?

[Snuffleupagus]

This feature has no "title" key in preferences_schema.json. This prevents the option from being rendered in the settings page of the Chrome extension.

That was completely intentional on my part. (It should still be possible to set the preference manually from the console, as far as I can tell.)

Preferences should only be hidden if they are internal, non-user facing or instable work-in-progress features. It seems that this feature does not match that description.

#10423 (comment) outlines some of the reasons why enabling this feature is a bad idea in my opinion, and thus I really don't think it's a good idea to give it a prominent position in any options UI.

If this feature is ready for use by users, could you open a PR and add a meaningful title to the preferences_schema.json, to expose the pref to users?

Honestly, I'd classify this as only "sort of" supported and thus use only at you're own risk.
Given everything said above, personally I have no intention to put additional work into this feature (and I only submitted this patch to prevent a solution causing outright breakage from landing).

[Rob--W]

Thanks for the quick reply. The reasoning sounds good to me.