Layout the sidebar in the same vertical position regardless of the viewer width (issue 4052, bug 850591)

[Snuffleupagus]

If we want to (eventually) make it possible to resize the sidebar, then having its width indirectly affect the toolbar is going to wreck havoc on the media queries used to show/hide buttons in the main toolbar (since many of them depend on the toolbar state, and thus its width).
Updating all of the media queries dynamically with JavaScript seems like a non-starter, given that it'd cause very messy code. It thus seem to me that we'd need to fix the position of the sidebar, to have any hope of (in the short term) addressing issue #2072.

Hence, I'm suggesting that the we always layout the sidebar in a consistent vertical position, and only animate the viewerContainer rather than the entire mainContainer.

Fixes #4052.
Fixes bug 850591.

[Snuffleupagus]

Based on this patch, I've attempted to implement sidebar resizing that's limited to modern browsers with support for CSS variables. The, proof-of-concept, patch is available at master...Snuffleupagus:sidebar-resize.

Based on the simplicity of the solution, I really think that we ought to consider if/why we need to go through the trouble of implementing a truly backwards compatible solution!?

[timvandermeij]

Indeed that looks really good to me. Personally I don't think we really need a completely backwards compatible solution. All modern browsers should support this just fine and if some really old IE version doesn't, I personally don't care since we're going to drop support for those old versions anyway. If @yurydelendik or @brendandahl are fine with this, we can proceed with this (and the other open patch).

[yurydelendik]

Is this patch still works (somehow) with old browsers? In this case maintains some accetable viewer appearance without breaking a layout too much.

P.S. If yes, we need to land it.

[Snuffleupagus]

Comment #8993 (comment) is somewhat orthogonal to the current PR; my apologies for causing unnecessary confusion here!

The purpose of this PR is to address a number of things that, as I see it, makes it much more difficult to implement a (maintainable) solution for the long-standing issue #2072.

Please note that there should be no compatibility breaking changes in this PR.

---

The major change here is simply that the sidebar is now placed in the same vertical position regardless of the viewer width, please see the comparison below.

The reason for this change is based on a number things:

• To avoid overflow in the (main) toolbar, a number of media queries are currently used to show/hide buttons as necessary.
  However, given the way that the sidebar currently works, we need specialized media queries for the "sidebar open" case. This is already increasing the maintenance burden of our CSS, but if sidebar resizing is implemented that becomes a nightmare to update dynamically.
  By only moving the viewerContainer, and not the entire mainContainer, the width of the toolbar no longer changes when the sidebar opens. Thus we can remove, and simplify, a lot of CSS.
• The vertical position of the sidebar in the current master is different depending on the viewer width (note what happens when the viewer is narrow), which also seems strange.
• The width of many of the elements in the sidebar is currently hard coded, which also makes solving issue Outline (bookmark) pane should be resizable. #2072 much harder. By using calc, many widths can be both simplified and better suited to arbitrary sidebar widths.
• Prevent the sidebar toggle button from moving around, which has always seemed a bit strange to me (and others as well, see https://bugzilla.mozilla.org/show_bug.cgi?id=850591).

[Snuffleupagus]

/botio-linux preview

[pdfjsbot]

From: Bot.io (Linux m4)

---

Received

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

Live output at: http://54.67.70.0:8877/b6e3eef29d6f0e3/output.txt

[pdfjsbot]

From: Bot.io (Linux m4)

---

Success

Full output at http://54.67.70.0:8877/b6e3eef29d6f0e3/output.txt

Total script time: 2.45 mins

Published

• Viewer: http://54.67.70.0:8877/b6e3eef29d6f0e3/web/viewer.html
• Extension: http://54.67.70.0:8877/b6e3eef29d6f0e3/extensions/firefox/pdf.js.xpi

[timvandermeij]

Nice work! Sorry for the delay in review by the way; I didn't have too much spare time lately and I wanted to test this well.