Issue #580 - "Disable on this page only" should discard the final # from URLs

This issue is related to current ticket:
https://issues.adblockplus.org/ticket/580

[saroyanm, 2014-06-11 16:33:38]

Wladimir can you please have a look when you have time, should be quick one,
Thanks in advance.

[Wladimir Palant, 2014-06-12 06:40:08]

http://codereview.adblockplus.org/5734888082767872/diff/5629499534213120/lib/...
File lib/ui.js (right):

http://codereview.adblockplus.org/5734888082767872/diff/5629499534213120/lib/...
lib/ui.js:1508: location.spec.substring(0, location.spec.length - 1) :
location.spec);
The code setting location.ref right below the one you added is responsible for
removing the hash part of the URL - you should fix that one rather than adding a
hack. Apparently, location.ref is "" both when the hash part is missing and when
it is just #. However, if we just assign an empty string to it without checking
whether it was already empty before it should always work.

For reference:
https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interfa...

[saroyanm, 2014-06-12 07:19:06]

Hope we can give a go now.

http://codereview.adblockplus.org/5734888082767872/diff/5629499534213120/lib/...
File lib/ui.js (right):

http://codereview.adblockplus.org/5734888082767872/diff/5629499534213120/lib/...
lib/ui.js:1508: location.spec.substring(0, location.spec.length - 1) :
location.spec);
On 2014/06/12 06:40:08, Wladimir Palant wrote:
> The code setting location.ref right below the one you added is responsible for
> removing the hash part of the URL - you should fix that one rather than adding
a
> hack. Apparently, location.ref is "" both when the hash part is missing and
when
> it is just #. However, if we just assign an empty string to it without
checking
> whether it was already empty before it should always work.
> 
> For reference:
>
https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interfa...

Good point, how I could miss that easy change.
Done.

[Wladimir Palant, 2014-06-12 08:09:36]

http://codereview.adblockplus.org/5734888082767872/diff/5649050225344512/lib/...
File lib/ui.js (right):

http://codereview.adblockplus.org/5734888082767872/diff/5649050225344512/lib/...
lib/ui.js:1507: location.ref = "";
Please don't remove |location instanceof Ci.nsIURL| check - it might be an
instance of nsIURI (e.g. data: URI), these don't have a hash part.

[saroyanm, 2014-06-12 14:20:39]

http://codereview.adblockplus.org/5734888082767872/diff/5649050225344512/lib/...
File lib/ui.js (right):

http://codereview.adblockplus.org/5734888082767872/diff/5649050225344512/lib/...
lib/ui.js:1507: location.ref = "";
On 2014/06/12 08:09:36, Wladimir Palant wrote:
> Please don't remove |location instanceof Ci.nsIURL| check - it might be an
> instance of nsIURI (e.g. data: URI), these don't have a hash part.

Done.

[Wladimir Palant, 2014-06-12 18:07:27]

LGTM