Added the data-page-number dom attribute to the page rendered in the browser

[mbbaig]

Added data-page-id attribute to page container divs

Updated the attribute with a better label

[timvandermeij]

Looks good. Fixes #5766. I do wonder though: now that we have this, do we still need the ID for every container (i.e., pageContainer1, pageContainer2, et cetera)? It seems like this data attribute is much better for querying and for the display styles we have the class, so is the ID even used?

[yurydelendik]

@timvandermeij we don't need it... unless somebody already started using/parsing it

[mbbaig]

is that not used anywhere within the library itself?

[yurydelendik]

I don't think so, it was just carried from demo viewer created at c828fdd

[timvandermeij]

Personally I think it is best to encourage developers to start using data-page-number instead of parsing the ID (which is a bit ugly anyway in my opinion), but I can also imagine that you do not want to get rid of it immediately. Shall we remove it or keep it for now?

[yurydelendik]

Let's remove

[mbbaig]

Okay good. Cause I already removed it and pushing the code.

[yurydelendik]

Found pageContainer usage at http://mxr.mozilla.org/mozilla-central/source/browser/extensions/pdfjs/test/browser_pdfjs_zoom.js , if we going to land this we need to change the test in m-c too

[mbbaig]

@yurydelendik So that is currently located here https://github.com/mozilla/gecko-dev/blob/master/browser/extensions/pdfjs/test/browser_pdfjs_zoom.js#L112 on GitHub. It that the repo that I should change?

[yurydelendik]

let's remove the latter change from this pr and open different issue to fix test first

[timvandermeij]

@mbbaig Could you change the commit such that only https://github.com/mbbaig/pdf.js/commit/9643750f02770ff6dbff0e0d8e13140fa7dc5a40#diff-c880b70cf7d6b97863eeb665a736c4baR82 remains? Maybe it's easiest to add back https://github.com/mbbaig/pdf.js/commit/9643750f02770ff6dbff0e0d8e13140fa7dc5a40#diff-c880b70cf7d6b97863eeb665a736c4baL79 and then squash the commits. With only the data-page-number addition this PR should be good to go; we can fix the ID removal with tests in a follow-up issue/PR.

[mbbaig]

I added the 'id' tag back so it can be associated with a different issue. The commits have also been cleaned up to only show the required addition.

[timvandermeij]

/botio-linux preview

[pdfjsbot]

From: Bot.io (Linux)

---

Received

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

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

[pdfjsbot]

From: Bot.io (Linux)

---

Success

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

Total script time: 0.79 mins

Published

• Viewer: http://107.21.233.14:8877/bc360a5ef3d875e/web/viewer.html
• B2G Viewer: http://107.21.233.14:8877/bc360a5ef3d875e/extensions/b2g/content/web/viewer.html
• Extension: http://107.21.233.14:8877/bc360a5ef3d875e/extensions/firefox/pdf.js.xpi

[timvandermeij]

Thank you for the patch!