Navigation to relative fragments does not work correctly for OOP pdf.

chrome/browser/resources/pdf/open_pdf_params_parser.js

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 'use strict';
 
 /**
  * Creates a new OpenPDFParamsParser. This parses the open pdf parameters
  * passed in the url to set initial viewport settings for opening the pdf.
  * @param {string} url to be parsed.
  */
 function OpenPDFParamsParser(url) {
-  this.url_ = url;
   this.urlParams = {};

[raymes, 2015/01/15 23:07:42]

let's remove this as an instance variable and make it a local variable

[Deepak, 2015/01/16 04:39:21]

On 2015/01/15 23:07:42, raymes wrote:

Done.

-  this.parseOpenPDFParams_();
+  // A dictionary of all the named destinations in the PDF.
+  this.namedDestinations = {};
 }
 
 OpenPDFParamsParser.prototype = {
   /**
    * @private
    * Parse zoom parameter of open PDF parameters. If this
    * parameter is passed while opening PDF then PDF should be opened
    * at the specified zoom level.
    * @param {number} zoom value.
    */
   parseZoomParam_: function(paramValue) {

[raymes, 2015/01/15 23:07:42]

This function can take the urlParams as an argument to make it a local variable

[Deepak, 2015/01/16 04:39:21]

On 2015/01/15 23:07:42, raymes wrote:

Done.

     var paramValueSplit = paramValue.split(',');
     if ((paramValueSplit.length != 1) && (paramValueSplit.length != 3))
       return;
 
     // User scale of 100 means zoom value of 100% i.e. zoom factor of 1.0.
     var zoomFactor = parseFloat(paramValueSplit[0]) / 100;
     if (isNaN(zoomFactor))
       return;
 
     // Handle #zoom=scale.
     if (paramValueSplit.length == 1) {
       this.urlParams['zoom'] = zoomFactor;
       return;
     }
 
     // Handle #zoom=scale,left,top.
     var position = {x: parseFloat(paramValueSplit[1]),
                     y: parseFloat(paramValueSplit[2])};
     this.urlParams['position'] = position;
     this.urlParams['zoom'] = zoomFactor;
   },
 
   /**
    * @private
-   * Parse open PDF parameters. These parameters are mentioned in the url
+   * Parse PDF url parameters. These parameters are mentioned in the url
    * and specify actions to be performed when opening pdf files.
    * See http://www.adobe.com/content/dam/Adobe/en/devnet/acrobat/
    * pdfs/pdf_open_parameters.pdf for details.
+   * @param {string} inputUrl that need to be parsed.

[raymes, 2015/01/15 23:07:43]

nit: need->needs
nit: inputUrl->url

[Deepak, 2015/01/16 04:39:21]

On 2015/01/15 23:07:43, raymes wrote:

Done.

+   * @return {Object} urlParams data that have the PDF url parameter info.

[raymes, 2015/01/15 23:07:42]

nit: @return {Object} A dictionary containing the viewport which should be
displayed based on the URL

[Deepak, 2015/01/16 04:39:21]

On 2015/01/15 23:07:42, raymes wrote:

Done.

    */
-  parseOpenPDFParams_: function() {
-    var originalUrl = this.url_;
+  getViewportFromUrlParams: function(inputUrl) {

[raymes, 2015/01/15 23:07:43]

nit: inputUrl->url

[Deepak, 2015/01/16 04:39:21]

On 2015/01/15 23:07:43, raymes wrote:

Done.

+    // Resetting urlParams.
+    this.urlParams = {};

[raymes, 2015/01/15 23:07:43]

var urlParams = {};

[Deepak, 2015/01/16 04:39:21]

On 2015/01/15 23:07:43, raymes wrote:

Done.

+    var originalUrl = inputUrl;

[raymes, 2015/01/15 23:07:42]

nit this isn't needed, just always use "url"

[Deepak, 2015/01/16 04:39:21]

On 2015/01/15 23:07:42, raymes wrote:

Done.

     var paramIndex = originalUrl.search('#');
     if (paramIndex == -1)
-      return;
+      return this.urlParams;
 
     var paramTokens = originalUrl.substring(paramIndex + 1).split('&');
+    if ((paramTokens.length == 1) && (paramTokens[0].search('=') == -1)) {

[raymes, 2015/01/15 23:08:37]

nit: Let's add the same comment here as in instance.cc (you might have had this
before but I got confused, sorry!!)

  // Handle the case of http://foo.com/bar#NAMEDDEST. This is not explicitly
  // mentioned except by example in the Adobe "PDF Open Parameters" document.

[Deepak, 2015/01/16 04:39:21]

On 2015/01/15 23:08:37, raymes wrote:

Done.

+      this.urlParams['page'] = this.namedDestinations[paramTokens[0]];
+      return this.urlParams;
+    }
+
     var paramsDictionary = {};
     for (var i = 0; i < paramTokens.length; ++i) {
       var keyValueSplit = paramTokens[i].split('=');
       if (keyValueSplit.length != 2)
         continue;
       paramsDictionary[keyValueSplit[0]] = keyValueSplit[1];
     }
 
+    if ('nameddest' in paramsDictionary) {
+      var pageNumber = parseInt(paramsDictionary['nameddest']);

[raymes, 2015/01/15 23:07:42]

paramsDictionary['nameddest'] isn't expected to be a number, it's expected to be
a named dest which we look up. e.g.

var page = paramsDictionary['nameddest']
if (page)
  this.urlParams['page'] = page;

[Deepak, 2015/01/16 04:39:21]

On 2015/01/15 23:07:42, raymes wrote:

Done.

+      if (!isNaN(pageNumber) && pageNumber >= 0)
+        this.urlParams['page'] = pageNumber;
+    }
+
     if ('page' in paramsDictionary) {
       // |pageNumber| is 1-based, but goToPage() take a zero-based page number.
       var pageNumber = parseInt(paramsDictionary['page']);
-      if (!isNaN(pageNumber))
+      if (!isNaN(pageNumber) && pageNumber > 0)
         this.urlParams['page'] = pageNumber - 1;
     }
 
     if ('zoom' in paramsDictionary)
       this.parseZoomParam_(paramsDictionary['zoom']);
+
+    return this.urlParams;
   }
 };