Remove page ID and fix tests at mozcentral

[timvandermeij]

In #5881 we have decided that we can now remove the id from each page div as it is unused and replaced by the more flexible data-page-number attribute. However, there is a test case at http://mxr.mozilla.org/mozilla-central/source/browser/extensions/pdfjs/test/browser_pdfjs_zoom.js that needs to be altered before we can remove this.

[awongCM]

Hi,

I'd like to take a crack at this, if that's okay with you.

Where should I start?

[timvandermeij]

Aside from creating a pull request that removes the id from each page div, a patch needs to be prepared to remove pageContainer mentions from http://mxr.mozilla.org/mozilla-central/source/browser/extensions/pdfjs/test/browser_pdfjs_zoom.js. We first need to fix the upstream tests before altering the code in this repository.

[awongCM]

Thanks @timvandermeij.

I'll make pull request tonight and get a feel of what's going behind the scenes on this request.

Cheers!

[mcorb]

It seems prudent to unblock ongoing web viewer work by making this preprocessor-conditional until someone at Mozilla can commit to removing the dependency entirely. See pull request #6050

[awongCM]

So how do I begin in preparing the fix for the upstream tests for browser_pdfjs_zoom.js file?

[yurydelendik]

@awongCM Simplest way is to have build of mozilla-central / firefox on local computer. See https://developer.mozilla.org/en-US/docs/Simple_Firefox_build and after the Firefox is build you can play with tests (see https://developer.mozilla.org/en-US/docs/Mochitest and https://developer.mozilla.org/en-US/docs/Browser_chrome_tests). Tests for PDF.js are located at http://mxr.mozilla.org/mozilla-central/source/browser/extensions/pdfjs/test/.

[awongCM]

Thanks @yurydelendik

I tried following the firefox_build instructions you gave me. As I'm using my Ubuntu Linux(13.0), I went over here.
https://developer.mozilla.org/en-US/docs/Simple_Firefox_build/Linux_and_MacOS_build_preparation.

I had trouble getting the system boostrapping to work, thus I did a number of troubleshooting steps till I reached this line.

sudo apt-get install zip unzip mercurial g++ make autoconf2.13 yasm libgtk2.0-dev libglib2.0-dev libdbus-1-dev libdbus-glib-1-dev libasound2-dev libcurl4-openssl-dev libiw-dev libxt-dev mesa-common-dev libgstreamer0.10-dev libgstreamer-plugins-base0.10-dev libpulse-dev m4 flex ccache

That failed as it saying something about Connection failed to one of the ubuntu linux servers http://ubuntu.mirror.uber.com.au

Then I attempted to do different commands with sudo apt-get update that failed to work only because of the failed connection as stated above, but because of the creepy error that stays stuck on "Waiting for headers".

andy@LINUXAWCM:~$ sudo apt-get update
[sudo] password for andy: 
Ign http://ubuntu.mirror.uber.com.au saucy InRelease
Ign http://ubuntu.mirror.uber.com.au saucy-updates InRelease
Ign http://ubuntu.mirror.uber.com.au saucy-backports InRelease
Ign http://ubuntu.mirror.uber.com.au saucy-security InRelease
Err http://ubuntu.mirror.uber.com.au saucy Release.gpg       
  Connection failed [IP: 113.20.13.217 80]
Err http://ubuntu.mirror.uber.com.au saucy-updates Release.gpg
  Connection failed [IP: 113.20.13.217 80]
Err http://ubuntu.mirror.uber.com.au saucy-backports Release.gpg
  Connection failed [IP: 113.20.13.217 80]
Err http://ubuntu.mirror.uber.com.au saucy-security Release.gpg
  Connection failed [IP: 113.20.13.217 80]
Ign http://ubuntu.mirror.uber.com.au saucy Release           
Ign http://ubuntu.mirror.uber.com.au saucy-updates Release   
Ign http://ubuntu.mirror.uber.com.au saucy-backports Release 
Ign http://ubuntu.mirror.uber.com.au saucy-security Release  
20% [Waiting for headers]       
.......................................
.......................................

I'm really stuck now. Dont' know what to do next..

[yurydelendik]

I had trouble getting the system boostrapping to work

Sorry to hear. But setting up the Firefox build on Linux is easier than on Mac OSX or Windows. Your issue, however, not related to Firefox build itself, so perhaps try the same steps again later. Or try setting up dependencies one-by-one.

[awongCM]

@yurydelendik - I'm using Linux for my Firefox build.

I'm aware it is Ubuntu Linux issue, which is the main blocker from getting bootstrapping to work.

I'll try googling for more answers tonight and see where else this may lead to.

Will keep you posted.

[awongCM]

@yurydelendik

I spent some time on this. With not much better luck.

I even contacted the main author of the bootstrapping site.

He tried assisting me in resolving this nagging issue. But none of us were successful in doing so.

I'm feeling there's little much we can do as we're both clueless how this happens.

I fear I may have to call it quits on this one unfortunately...

Sigh

[mcorb]

Shall I resurrect #6050? We don't use the Firefox extension here so condititionalizing the legacy code works perfectly for us, and I believe the patch will meet all use cases including those of the Netscape team.

[davehouse]

@yurydelendik I r?'d you on https://bugzilla.mozilla.org/show_bug.cgi?id=1331795 for switching the mozilla-central test to using the data-page-number attribute.

If I'm doing that correctly, then I'd like to get that in and then PR the changes in pdf.js to not set the id. Is that still valid/desired?

[timvandermeij]

Yes, it certainly is. The upstream patch has been merged, so if you could also create a PR for this repository, that would be great.

[yurydelendik]

so if you could also create a PR for this repository, that would be great.

I don't think we have mozcentral tests in our local repo. The upsteam bug marked as resolved. Closing the issue.

[timvandermeij]

The intent of this issue is to remove https://github.com/mozilla/pdf.js/blob/master/web/pdf_page_view.js#L115. For that, the upstream test needed to be fixed first, for which you're right that that has been done now. What still remains is the removal of that line, as discussed in #5881, since it's obsolete, so this issue is not resolved yet.

[yurydelendik]

I see, thanks