Modify a number of the viewer preferences, whose current default value is 0, such that they behave as expected with the view history
#10502
Conversation
Snuffleupagus
force-pushed
the
adjust-onLoad-prefs
branch
7 times, most recently
from
fbfb4da
to
35f580f
Compare
Snuffleupagus
force-pushed
the
adjust-onLoad-prefs
branch
from
35f580f
to
0bc495d
Compare
Snuffleupagus
changed the title
0, such that they behave as expected with the view history0, such that they behave as expected with the view history
|
/botio-linux preview |
Snuffleupagus
force-pushed
the
adjust-onLoad-prefs
branch
2 times, most recently
from
1b94a42
to
0ec4d53
Compare
…DFSidebar._switchView` method The new "private" method will return a boolean, indicating if the `sidebarviewchanged` event was dispatched, thus allowing some simplification of the `PDFSidebar.setInitialView` method.
…e is `0`, such that they behave as expected with the view history The intention with preferences such as `sidebarViewOnLoad`/`scrollModeOnLoad`/`spreadModeOnLoad` were always that they should be able to *unconditionally* override their view history counterparts. Due to the way that these preferences were initially implemented[1], trying to e.g. force the sidebar to remain hidden on load cannot be guaranteed[2]. The reason for this is the use of "enumeration values" containing zero, which in hindsight was an unfortunate choice on my part. At this point it's also not as simple as just re-numbering the affected structures, since that would wreak havoc on existing (modified) preferences. The only reasonable solution that I was able to come up with was to change the *default* values of the preferences themselves, but not their actual values or the meaning thereof. As part of the refactoring, the `disablePageMode` preference was combined with the *adjusted* `sidebarViewOnLoad` one, to hopefully reduce confusion by not tracking related state separately. Additionally, the `showPreviousViewOnLoad` and `disableOpenActionDestination` preferences were combined into a *new* `viewOnLoad` enumeration preference, to further avoid tracking related state separately.
… preferences to the new `viewOnLoad` preference This patch ignores the recently added `disableOpenActionDestination` preference, since the latest PDF.js version found on the "Chrome Web Store" doesn't include it.
…erApplication` This avoids having the initialization code "spread out", and will become even simpler once the `TODO` is addressed (which I'm planning on fixing as soon as possible).
The intention with this change is to, more clearly, highlight when the default values may possibly be overridden by "compatibility" values.
Snuffleupagus
force-pushed
the
adjust-onLoad-prefs
branch
from
0ec4d53
to
ef634b5
Compare
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/fa2fbf7e8e5276a/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/fa2fbf7e8e5276a/output.txt Total script time: 1.70 mins Published |
|
I notice there is migration code for the Chrome extension, but how will the removal of the old preferences work for the version embedded in Firefox? Don't we need some migration code for that as well, i.e., temporarily handle both the old and the new preferences? |
Honestly, I was kind of hoping not having to bother with general preference migration[1], since it's only in the Chrome extension that the preferences are easily accessible.
If you insist on migration being necessary here, I really cannot object too strongly though[2]. [1] Which unfortunately the [2] Most likely, with my review hat on, I would've asked the exact same questions. |
|
I agree with this. Changing preferences through |
|
Thank you for improving this! |
The intention with preferences such as
sidebarViewOnLoad/scrollModeOnLoad/spreadModeOnLoadwas always that they should be able to unconditionally override their view history counterparts.Due to the way that these preferences were initially implemented[1], trying to e.g. force the sidebar to remain hidden on load cannot be guaranteed[2]. The reason for this is the use of "enumeration values" containing zero, which in hindsight was an unfortunate choice on my part.
At this point it's also not as simple as just re-numbering the affected structures, since that would wreak havoc on existing (modified) preferences. The only reasonable solution that I was able to come up with was to change the default values of the preferences themselves, but not their actual values or the meaning thereof.
As part of the refactoring, the
disablePageModepreference was combined with the adjustedsidebarViewOnLoadone, to hopefully reduce confusion by not tracking related state separately.Additionally, the
showPreviousViewOnLoadanddisableOpenActionDestinationpreferences were combined into a newviewOnLoadenumeration preference, to further avoid tracking related state separately.[1] This is mostly my fault, since I wrote the initial implementation which has then been used as a template when additional preferences were added.
I've been aware of these problems for quite a while now, and seing issue #10335 provided the final impetus needed to actually try and fix this.
[2] Given that a
ViewHistoryvalue may easily override the relevantPreferencevalue.