Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(15)

Issue 1267553004: Revert of Add a scroll offset to PDF documents to account for the top material design toolbar. (Closed)

Created:
5 years, 4 months ago by Vitaly Buka (NO REVIEWS)
Modified:
5 years, 4 months ago
CC:
chromium-reviews, arv+watch_chromium.org, chrome-apps-syd-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Add a scroll offset to PDF documents to account for the top material design toolbar. (patchset #7 id:120001 of https://codereview.chromium.org/1255403002/ ) Reason for revert: This patch breaks print preview. BUG=516829 Original issue's description: > Add a scroll offset to PDF documents to account for the top material design toolbar. > > Previously the toolbar in the material design PDF UI would always cover the top > of pages when it was first loaded or when a page was navigated to using the > page selector. Now we ensure that a blank region is left at the very top of > the document when it is first loaded. This is the region that the toolbar > covers, so the document is not obscured at all. When pages are navigated to, we > ensure that the top of the selected page is always underneath the toolbar > so that it is not obscured. The one exception to this is when in fit-to-page > mode which causes the page to be zoomed to cover the entire screen, ignoring > the toolbar. This is so that users can take advantaging of filling all of the > screen real-estate with a page when that is what they want. > > This is implemented by initially scrolling the document to a negative offset > (which is equal to the toolbar height). All subsequent scrolls are relative > to this initial scroll. A few small bugs that assumed there was no blank space > above the first page have also been fixed. > > BUG=439114 > > Committed: https://crrev.com/daad0f1f879b13c8b55797ae5ce106d382283047 > Cr-Commit-Position: refs/heads/master@{#341685} TBR=sammc@chromium.org,tsergeant@chromium.org,raymes@chromium.org BUG=439114 Committed: https://crrev.com/0bb69c7f9ab18c6e79dd607ee6ab405231964dac Cr-Commit-Position: refs/heads/master@{#341820}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+44 lines, -148 lines) Patch
M chrome/browser/resources/pdf/pdf.js View 3 chunks +2 lines, -14 lines 0 comments Download
M chrome/browser/resources/pdf/viewport.js View 10 chunks +16 lines, -37 lines 0 comments Download
M chrome/test/data/pdf/basic_plugin_test.js View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/test/data/pdf/navigator_test.js View 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/data/pdf/viewport_test.js View 11 chunks +11 lines, -52 lines 0 comments Download
M pdf/out_of_process_instance.h View 1 chunk +0 lines, -4 lines 0 comments Download
M pdf/out_of_process_instance.cc View 7 chunks +9 lines, -25 lines 0 comments Download
M pdf/pdf_engine.h View 1 chunk +0 lines, -3 lines 0 comments Download
M pdf/pdfium/pdfium_engine.h View 2 chunks +4 lines, -1 line 0 comments Download
M pdf/pdfium/pdfium_engine.cc View 1 chunk +0 lines, -9 lines 0 comments Download

Messages

Total messages: 5 (1 generated)
Vitaly Buka (NO REVIEWS)
Created Revert of Add a scroll offset to PDF documents to account for the top ...
5 years, 4 months ago (2015-08-04 21:42:51 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1267553004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1267553004/1
5 years, 4 months ago (2015-08-04 21:44:19 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 4 months ago (2015-08-04 23:20:20 UTC) #4
commit-bot: I haz the power
5 years, 4 months ago (2015-08-04 23:20:57 UTC) #5
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/0bb69c7f9ab18c6e79dd607ee6ab405231964dac
Cr-Commit-Position: refs/heads/master@{#341820}

Powered by Google App Engine
This is Rietveld 408576698