Move punycode.js to correct position for lib/adblockplus.js

[kzar]

Environment

Adblock Plus for Google Chrome 1.9.4.1498

How to reproduce

1. Open the options page.
2. Enter location.href = "qunit/index.html"; in the console.

Observed behaviour

Some tests fail with the message "Cannot read property 'toUnicode' of undefined".

Expected behaviour

Those tests should not fail.

Notes

• It turns out the ordering for the lib/adblockplus.js [convert_js] section in adblockpluschrome/metadata.common is wrong. lib/punycode.js must be above lib/url.js.
• This bug is also visible to the user when not using content blocking if the URL of a request or document uses an IDN domain, in which case the same exception is thrown and adverts are not blocked.

Hints for testers

1. Open the console for the Adblock Plus background page. (Open extensions menu, enable developer mode and click "background page".)
2. Browse to http://bücher.com (will will become http://xn--bcher-kva.com)
3. Ensure that there are not TypeError: Cannot read property 'toUnicode' of undefined exceptions being thrown in the background page console.

[abpbot]

A commit referencing this issue has landed:
​Issue 4089 - Fix punycode module ordering

[Ross]

Running the above in the console results in lots of (pointing at various files):

chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/qunit/common.js net::ERR_FILE_NOT_FOUND

Should I be testing this in the Chrome extension or something else? What is Adblock Update?

ABP 1.11.0.1610
Chrome 48 / Ubuntu 14.04

[kzar]

I'm not sure what's happening there, the tests all pass for me. I'm going to try installing that development build now.

What is Adblock Update?

A typo, fixed now.

Should I be testing this in the Chrome extension or something else?

Yes in the Chrome extension.

[kzar]

Hang on, which files are failing to load? I see some in the console that are just caused by the fact that the tests live inside the qunit directory. (subscriptions.xml, abp-19.png and abp-38.png.) If that's all you're seeing too then I don't think it's cause for concern.

[Ross]

It's this list (ignoring the icons):

index.html:12 GET chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/qunit/qunit.css net::ERR_FILE_NOT_FOUND
index.html:13 GET chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/qunit/qunit.js net::ERR_FILE_NOT_FOUND
index.html:23 GET chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/qunit/common.js net::ERR_FILE_NOT_FOUND
index.html:24 GET chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/qunit/tests/adblockplus.js net::ERR_FILE_NOT_FOUND
index.html:25 GET chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/qunit/tests/versionComparator.js net::ERR_FILE_NOT_FOUND
index.html:26 GET chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/qunit/tests/url.js net::ERR_FILE_NOT_FOUND
index.html:27 GET chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/qunit/tests/signatures.js net::ERR_FILE_NOT_FOUND
index.html:28 GET chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/qunit/tests/cssEscaping.js net::ERR_FILE_NOT_FOUND

[kzar]

OK I can also reproduce this when installing development builds from the website, I tried 1.11.0.1610, 1.11.0.1609, 1.11.0.1608 and 1.11.0.1607. Almost all the qunit files fail to load as Ross mentioned and the test page is pretty much completely broken.

I can't reproduce this when installing from a local devenv copy though, from there the tests run and now all pass.

My only guess is that some of the test files are deliberately not included in the builds to save space. I'm not sure about that though, hopefully Wladimir / Sebastian can confirm.

[kzar]

(In the hope that the problem running the qunit tests is to be expected I've added some alternative testing instructions.)

[trev]

Yes, unit tests aren't meant to be run from the extension builds, merely from the development environment. Nor should testing an issue be done via unit tests, it should rather be reproduced in the real world. And in fact, when going back to a build where the issue isn't fixed yet I see lots of instances of that error message in the background page after navigating to ​http://россия.рф/.

The fact that qunit/index.html is included in the builds is actually a bug.

[trev]

I filed #4096 on that bug.