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

Unified Diff: chrome/browser/resources/pdf/pdf.js

Issue 830433002: Navigation to relative fragments does not work correctly for OOP pdf. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Changes as per review comments. Created 5 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/resources/pdf/pdf.js
diff --git a/chrome/browser/resources/pdf/pdf.js b/chrome/browser/resources/pdf/pdf.js
index 735c508fdd197c0924cf7b8127f5f9d6c0eb9ee0..2ee6b265e0b2497a73ca13827ff0c11750442daf 100644
--- a/chrome/browser/resources/pdf/pdf.js
+++ b/chrome/browser/resources/pdf/pdf.js
@@ -136,8 +136,7 @@ function PDFViewer(streamDetails) {
}
// Parse open pdf parameters.
- var paramsParser = new OpenPDFParamsParser(this.streamDetails_.originalUrl);
- this.urlParams_ = paramsParser.urlParams;
+ this.paramsParser_ = new OpenPDFParamsParser();
}
PDFViewer.prototype = {
@@ -284,17 +283,20 @@ PDFViewer.prototype = {
* important as later actions can override the effects of previous actions.
*/
handleURLParams_: function() {
- if (this.urlParams_.page)
- this.viewport_.goToPage(this.urlParams_.page);
- if (this.urlParams_.position) {
+ var urlParams =
+ this.paramsParser_.getViewportFromUrlParams(
+ this.streamDetails_.originalUrl);
+ if (urlParams.page)
+ this.viewport_.goToPage(urlParams.page);
+ if (urlParams.position) {
// Make sure we don't cancel effect of page parameter.
this.viewport_.position = {
- x: this.viewport_.position.x + this.urlParams_.position.x,
- y: this.viewport_.position.y + this.urlParams_.position.y
+ x: this.viewport_.position.x + urlParams.position.x,
+ y: this.viewport_.position.y + urlParams.position.y
};
}
- if (this.urlParams_.zoom)
- this.viewport_.setZoom(this.urlParams_.zoom);
+ if (urlParams.zoom)
+ this.viewport_.setZoom(urlParams.zoom);
},
/**
@@ -341,6 +343,64 @@ PDFViewer.prototype = {
/**
* @private
+ * Helper function to navigate to given URL. This might involve navigating
+ * within the PDF page or opening a new url (in the same tab or a new tab)
raymes 2015/01/15 23:07:43 nit: '.' at the end of hte sentence
Deepak 2015/01/16 04:39:21 On 2015/01/15 23:07:43, raymes wrote: Done.
+ * @param {string} message data that have the url for navigation.
raymes 2015/01/15 23:07:43 You need to include the name of the param here. e.
Deepak 2015/01/16 04:39:21 On 2015/01/15 23:07:43, raymes wrote: Done.
+ * @param {boolean} message data that have info regarding navigation happen
+ * within the PDF page or in the newTab.
+ */
+ navigate_: function(url, isNewTab) {
raymes 2015/01/15 23:07:43 nit: isNewTab->newTab
Deepak 2015/01/16 04:39:21 On 2015/01/15 23:07:43, raymes wrote: Done.
+ if (url.length == 0)
+ return;
+ var originalUrl = this.streamDetails_.originalUrl;
+ // If |urlFragment| starts with '#', then it's for the same URL with a
+ // different URL fragment.
+ if (url.charAt(0) == '#') {
+ // if '#' is already present in |originalUrl| then remove old fragment
+ // and add new url fragment.
+ var hashIndex = originalUrl.search('#');
+ if (hashIndex != -1)
+ url = originalUrl.substring(0, hashIndex) + url;
+ else
+ url = originalUrl + url;
+ }
+
+ var inputURL = url;
+ // If there's no scheme, add http.
+ if (inputURL.indexOf('://') == -1 && inputURL.indexOf('mailto:') == -1)
+ inputURL = 'http://' + inputURL;
+
+ // Make sure inputURL starts with a valid scheme.
+ if (inputURL.indexOf('http://') != 0 &&
+ inputURL.indexOf('https://') != 0 &&
+ inputURL.indexOf('ftp://') != 0 &&
+ inputURL.indexOf('file://') != 0 &&
+ inputURL.indexOf('mailto:') != 0) {
+ return;
+ }
+ // Make sure inputURL is not only a scheme.
+ if (inputURL == 'http://' ||
+ inputURL == 'https://' ||
+ inputURL == 'ftp://' ||
+ inputURL == 'file://' ||
+ inputURL == 'mailto:') {
+ return;
+ }
+
+ if (isNewTab) {
+ chrome.tabs.create({ url: inputURL });
+ } else {
+ var pageNumber =
+ this.paramsParser_.getViewportFromUrlParams(inputURL).page;
+ if (pageNumber != undefined)
+ this.viewport_.goToPage(pageNumber);
+ else
+ window.location.href = inputURL;
+ }
+ },
+
+ /**
+ * @private
* An event handler for handling message events received from the plugin.
* @param {MessageObject} message a message event.
*/
@@ -386,10 +446,10 @@ PDFViewer.prototype = {
this.updateProgress_(message.data.progress);
break;
case 'navigate':
- if (message.data.newTab)
- window.open(message.data.url);
- else
- window.location.href = message.data.url;
+ this.navigate_(message.data.url, message.data.newTab);
+ break;
+ case 'setNamedDestinations':
+ this.paramsParser_.namedDestinations = message.data.namedDestinations;
break;
case 'setScrollPosition':
var position = this.viewport_.position;

Powered by Google App Engine
This is Rietveld 408576698