[api-minor] Add support for PageLabels in the API #6803

Snuffleupagus · 2015-12-26T18:00:27Z

This PR adds support for PageLabels in the API.

TODO:

Decide if we actually want this feature in PDF.js.
~~Hook this functionality into the viewer.~~
Add unit-tests.
Improve the code (I hacked this together quickly while having a slight fever, so improvements should be possible :)

Re: #3165 and https://bugzilla.mozilla.org/show_bug.cgi?id=793632.

timvandermeij · 2015-12-26T20:14:54Z

I definitely think that this would be a nice feature to include in PDF.js, especially since it is used in practice and test cases are available. I usually see this kind of page numbering in e-books.

With regards to hooking this into the viewer, we must look into how other readers handle this in their page number box, i.e., if a user enters page 1, will he arrive at the page with page label 1 or at the first page of the document?

I have added some comments to improve the code, but overall it's already looking good. I really like the minimal changes required to re-use the NameTree structure for the NumberTree structure.

timvandermeij · 2015-12-26T20:17:50Z

src/core/obj.js

+
+          switch (style) {
+            case 'D':
+              currentLabel = '' + currentIndex;


When we reset the label at the end of the loop, we might be able to simplify this to currentLabel += currentIndex, according to a quick fiddle I made at https://jsfiddle.net/evyujauw/1/.

timvandermeij · 2015-12-28T23:37:55Z

There has been some discussion about this on IRC, starting here: http://logs.glob.uno/?c=mozilla%23pdfjs#c43831

The API for this appears fine, but we need to carefully determine how to hook this into the viewer. Adobe's behavior can be confusing, so we need to implement this in a way that makes more sense. Let's think about ideas to realize this.

yurydelendik · 2015-12-28T23:59:25Z

I'm thinking there is mistake at "Uppercase letters (A to Z for the first 26 pages, AA to ZZ for the next 26, and so on) " at http://www.adobe.com/content/dam/Adobe/en/devnet/acrobat/pdfs/PDF32000_2008.pdf#page=383&zoom=100,-178,800

I'm thinking they meant "... AA to ZZ for the next 26*26 (676)... " -- it goes as AA, AB, ... AZ, BA, ... ZZ

Snuffleupagus · 2015-12-29T00:32:53Z

I'm thinking they meant "... AA to ZZ for the next 26*26 (676)... " -- it goes as AA, AB, ... AZ, BA, ... ZZ

You might think so, but that's not how it's implemented in Adobe Reader, which seems to follow the specification to the letter (sorry, couldn't resist the pun :-).
I've modified a long document (the PDF spec) to use that kind of page labels, please see https://www.dropbox.com/s/9z7e37sydrrxvb0/PDF_spec.pdf?dl=1 (but beware of the file size, I wasn't able to compress it well enough).

yurydelendik · 2015-12-29T00:35:46Z

Oh, wow! That's so .... unorthodox.

Snuffleupagus · 2016-01-07T14:20:17Z

Given that the API/core parts of this are pretty self-contained, and have unit-tests, could we consider landing this as-is and then figure out how/if we should wire this into the default viewer in a follow-up?

In my opinion, having the ability to at least fetch pageLabels in the API seems like a good thing regardless of what we decide to do for the default viewer.

Snuffleupagus · 2016-01-12T11:18:19Z

The test commit was removed, the following code can be used in the console to fetch the pageLabels:

PDFViewerApplication.pdfDocument.getPageLabels().then(function (labels) {
  console.log(labels);
});

timvandermeij · 2016-01-19T21:39:46Z

src/shared/util.js

+
+    var romanStr = romanBuf.join('');
+    return (lowerCase ? romanStr.toLowerCase() : romanStr);
+};


This line does not appear to be indented correctly.

Whoops, fixed now. Thanks for spotting this.

timvandermeij · 2016-01-19T21:40:02Z

Could you rebase this patch and fix the comment above? We can land the API first and file a follow-up issue for hooking it into the viewer: see http://logs.glob.uno/?c=mozilla%23pdfjs&s=28+Dec+2015&e=28+Dec+2015&h=api#c43853.

timvandermeij · 2016-01-20T20:05:07Z

/botio-linux preview

pdfjsbot · 2016-01-20T20:05:07Z

From: Bot.io (Linux)

Received

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

Live output at: http://107.21.233.14:8877/de07095a1154df9/output.txt

pdfjsbot · 2016-01-20T20:05:59Z

From: Bot.io (Linux)

Success

Full output at http://107.21.233.14:8877/de07095a1154df9/output.txt

Total script time: 0.85 mins

Published

timvandermeij · 2016-01-20T20:06:08Z

/botio test

pdfjsbot · 2016-01-20T20:06:09Z

From: Bot.io (Linux)

Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://107.21.233.14:8877/ec6c40bf43540bd/output.txt

pdfjsbot · 2016-01-20T20:06:09Z

From: Bot.io (Windows)

Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://107.22.172.223:8877/868e91673424a49/output.txt

pdfjsbot · 2016-01-20T20:27:32Z

From: Bot.io (Linux)

Success

Full output at http://107.21.233.14:8877/ec6c40bf43540bd/output.txt

Total script time: 21.37 mins

Font tests: Passed
Unit tests: Passed
Regression tests: Passed

pdfjsbot · 2016-01-20T20:27:38Z

From: Bot.io (Windows)

Success

Full output at http://107.22.172.223:8877/868e91673424a49/output.txt

Total script time: 21.49 mins

Font tests: Passed
Unit tests: Passed
Regression tests: Passed

[api-minor] Add support for PageLabels in the API

timvandermeij · 2016-01-20T21:06:29Z

This API looks really good! I'll file a follow-up issue for deciding if/how we want to hook this into the viewer.

Snuffleupagus added core 4-work-in-progress labels Dec 26, 2015

timvandermeij reviewed Dec 26, 2015
View reviewed changes

timvandermeij added the feature label Dec 26, 2015

Snuffleupagus removed the 4-work-in-progress label Jan 7, 2016

Snuffleupagus changed the title ~~[WIP][api-minor] Add support for PageLabels in the API~~ [api-minor] Add support for PageLabels in the API Jan 7, 2016

timvandermeij reviewed Jan 19, 2016
View reviewed changes

Snuffleupagus added 2 commits January 19, 2016 22:47

Add support for NumberTree

8ad1895

[api-minor] Add support for PageLabels in the API

85cf906

timvandermeij added a commit that referenced this pull request Jan 20, 2016

Merge pull request #6803 from Snuffleupagus/page-labels

58329f7

[api-minor] Add support for PageLabels in the API

timvandermeij merged commit 58329f7 into mozilla:master Jan 20, 2016

timvandermeij mentioned this pull request Jan 20, 2016

Hook page labels into the viewer #6902

Closed

SamyCookie mentioned this pull request Jan 21, 2016

API support for page labels #3165

Closed

Snuffleupagus deleted the page-labels branch January 21, 2016 19:48

Snuffleupagus mentioned this pull request Jan 26, 2016

[api-minor] Change getPageLabels to take a boolean value to indicate if standard page numbering should be skipped #6923

Closed

Yetangitu mentioned this pull request Feb 27, 2018

Can't open a specific PDF page Yetangitu/owncloud-apps#83

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[api-minor] Add support for PageLabels in the API #6803

[api-minor] Add support for PageLabels in the API #6803

Snuffleupagus commented Dec 26, 2015

timvandermeij commented Dec 26, 2015

timvandermeij Dec 26, 2015

timvandermeij commented Dec 28, 2015

yurydelendik commented Dec 28, 2015

Snuffleupagus commented Dec 29, 2015

yurydelendik commented Dec 29, 2015

Snuffleupagus commented Jan 7, 2016

Snuffleupagus commented Jan 12, 2016

timvandermeij Jan 19, 2016

Snuffleupagus Jan 19, 2016

timvandermeij commented Jan 19, 2016

timvandermeij commented Jan 20, 2016

pdfjsbot commented Jan 20, 2016

pdfjsbot commented Jan 20, 2016

timvandermeij commented Jan 20, 2016

pdfjsbot commented Jan 20, 2016

pdfjsbot commented Jan 20, 2016

pdfjsbot commented Jan 20, 2016

pdfjsbot commented Jan 20, 2016

timvandermeij commented Jan 20, 2016

[api-minor] Add support for PageLabels in the API #6803

[api-minor] Add support for PageLabels in the API #6803

Conversation

Snuffleupagus commented Dec 26, 2015

timvandermeij commented Dec 26, 2015

timvandermeij Dec 26, 2015

Choose a reason for hiding this comment

timvandermeij commented Dec 28, 2015

yurydelendik commented Dec 28, 2015

Snuffleupagus commented Dec 29, 2015

yurydelendik commented Dec 29, 2015

Snuffleupagus commented Jan 7, 2016

Snuffleupagus commented Jan 12, 2016

timvandermeij Jan 19, 2016

Choose a reason for hiding this comment

Snuffleupagus Jan 19, 2016

Choose a reason for hiding this comment

timvandermeij commented Jan 19, 2016

timvandermeij commented Jan 20, 2016

pdfjsbot commented Jan 20, 2016

From: Bot.io (Linux)

Received

pdfjsbot commented Jan 20, 2016

From: Bot.io (Linux)

Success

Published

timvandermeij commented Jan 20, 2016

pdfjsbot commented Jan 20, 2016

From: Bot.io (Linux)

Received

pdfjsbot commented Jan 20, 2016

From: Bot.io (Windows)

Received

pdfjsbot commented Jan 20, 2016

From: Bot.io (Linux)

Success

pdfjsbot commented Jan 20, 2016

From: Bot.io (Windows)

Success

timvandermeij commented Jan 20, 2016