Smile4ever commented 6 years ago

@crssi

The code is in git, plus there is also a compiled version in ZIP format (but I think it's easier to use about:debugging)

Things you can test:

Several encoding bugs fixed #73 #75 #93
Re-initialise listener, hopefully fixes #92
Fix "Show counter" setting (introduced in 3.2.0) doesn't do anything #91
Workaround for gws_rd@google.* on google.com #76
Implement hash parameters using #? #83

And any breakages you notice. Please use this issue for any problems you find that are unrelated to existing issues (otherwise, use the relevant issue to comment on).

crssi commented 6 years ago

Wildcards doesn't work anymore. Add pf_rd_* to blocked parameters and visit http://www.imdb.com/title/tt1507571/?pf_rd_m=A2FGELUUNOQJNL&pf_rd_p=2413b25e-e3f6-4229-9efd-599bb9ab1f97&pf_rd_r=1WAXMJ8WV3Z3G3V5RN6D&pf_rd_s=right-2&pf_rd_t=15061&pf_rdi=homepage&ref=hm_otw_t3

If you add also ref_ after pf_rd_*, the even ref_ will not work anymore.

crssi commented 6 years ago

https:/somedomain.com/weservice.exe?SID=1255165669+FID=5234+OP=SEARCH+FACTORY=cr+KEEP.where_clause=%28status%20like%20%27RE%25%27%20OR%20status%20like%20%27%25comp%25%27%20OR%20status%20%3D%27Z_CPL%27%29%20AND%20tenant.name%3D%27ServiceDesk%27%20

gets rewritten to: https://somedomain.com/weservice.exe?SID=1255165669+FID=5234+OP=SEARCH+FACTORY=cr+KEEP.where_clause=(status like 'RE%25' OR status like '%25comp%25' OR status %3D'Z_CPL') AND tenant.name%3D'ServiceDesk'

But it should stayed intact (no changes should be made)

crssi commented 6 years ago

Re-initialise listener seems to work OK.

Smile4ever commented 6 years ago

But it should stayed intact (no changes should be made)

Is this a regression since 4.0.0 or is it the case for the current Neat URL version on addons.mozilla.org as well?

Smile4ever commented 6 years ago

Task list for Smile4ever

[x] Fix wildcard support
[x] Look at URL encoding again, reset the URL portion of the URL if it's still the same
[x] Logging: no record of where change was made

crssi commented 6 years ago

I can't see changes in the browser console. Only [Neat URL]: no params for entries are listed there, but no record of where change was made.

crssi commented 6 years ago

Fix "Show counter" setting (introduced in 3.2.0) doesn't do anything

Looks fixed now.

crssi commented 6 years ago

Click on Neat URL icon on toolbar does not switch between disabled and enabled.

crssi commented 6 years ago

Implement hash parameters using #? #83 doesn't seem to be resolved.

crssi commented 6 years ago

@Smile4ever thank you for your work. If you need any additional testing, just let me know.

Cheers

Smile4ever commented 6 years ago

Looks fixed now.

Thanks

Click on Neat URL icon on toolbar does not switch between disabled and enabled.

Sometimes the badge text interferes with the enable/disable functionality, I'm afraid this is a bug in Firefox and not something Neat URL can change.

Implement hash parameters using #? #83 doesn't seem to be resolved.

How did you test this? I used https://vivaldi.com/blog/teamblog/vivaldi-1-13-adds-window-panel/#pk_campaign=newsletter&pk_kwd=community

Smile4ever commented 6 years ago

Regarding the logging:

[Neat URL]: (type main_frame): 'http://www.omgubuntu.co.uk/2017/05/ubuntu-desktop-gnome-extensions-survey-1710?utm_source=feedburner&utm_medium=feed&utm_campaign=Feed%3A+d0od+(OMG!+Ubuntu!)' has been changed to 'http://www.omgubuntu.co.uk/2017/05/ubuntu-desktop-gnome-extensions-survey-1710' background.js:462:3 [Neat URL]: no params for 'http://www.omgubuntu.co.uk/2017/05/ubuntu-desktop-gnome-extensions-survey-1710' background.js:410:24

It seems to work okay and as expected.

Smile4ever commented 6 years ago

Wildcards doesn't work anymore.

Can you retest using the changes in git? (not the ZIP version, that one wasn't updated yet)

But it should stayed intact (no changes should be made)

Can you retest using the changes in git? (not the ZIP version, that one wasn't updated yet)

crssi commented 6 years ago

That is strange... I retested all again and still the same. I have tried with https://github.com/Smile4ever/firefoxaddons/blob/master/Neat URL-webext/web-ext-artifacts/neat_url-4.0.0.zip

and with https://github.com/Smile4ever/firefoxaddons/archive/master.zip

Can you point me to the git directly?... maybe I am taking the wrong source.

Cheers

Smile4ever commented 6 years ago

Assuming you're on Windows, you can install Git for Windows: https://git-for-windows.github.io/
After the installation, you start a Git Bash window.
You type the following command: git clone https://github.com/Smile4ever/firefoxaddons
Then you will have the most recent version for sure.

In case you don't manage to do the steps above: I also updated the ZIP version which can be found here: https://github.com/Smile4ever/firefoxaddons/blob/master/Neat%20URL-webext/web-ext-artifacts/neat_url-4.0.0.zip

crssi commented 6 years ago

OK... I will take the ZIP. It seems easier. :)

crssi commented 6 years ago

But it should stayed intact (no changes should be made)

Is this a regression since 4.0.0 or is it the case for the current Neat URL version on addons.mozilla.org as well?

Its different output between 3.2.0 and 4.0.0...

Original: https:/somedomain.com/weservice.exe?SID=1255165669+FID=5234+OP=SEARCH+FACTORY=cr+KEEP.where_clause=%28status%20like%20%27RE%25%27%20OR%20status%20like%20%27%25comp%25%27%20OR%20status%20%3D%27Z_CPL%27%29%20AND%20tenant.name%3D%27ServiceDesk%27%20

3.2.0: https:/somedomain.com/weservice.exe?SID=1255165669+FID=5234+OP=SEARCH+FACTORY=cr+KEEP.where_clause=%28status+like+%27RE%25%27+OR+status+like+%27%25comp%25%27+OR+status+=%27Z_CPL%27%29+AND+tenant.name=%27ServiceDesk%27+

4.0.0: https://somedomain.com/weservice.exe?SID=1255165669+FID=5234+OP=SEARCH+FACTORY=cr+KEEP.where_clause=(status like 'RE%25' OR status like '%25comp%25' OR status %3D'Z_CPL') AND tenant.name%3D'ServiceDesk'

crssi commented 6 years ago

Took the latest ZIP, but still the same, at least the your vivaldi test URL, but wildcards are now OK (the pf_rd_* sample). I have to quit for today. Will try with a clean profile again tomorrow afternoon.

Cheers

Smile4ever commented 6 years ago

Thank you. Are you sure you added "#?pk_campaign" (with the question mark) and not "#pk_campaign"?

crssi commented 6 years ago

^^That was it. "Vivaldi" is OK now.

crssi commented 6 years ago

Logging also OK.

crssi commented 6 years ago

Now I have lost myself what I have tried and what not. :) Will go over all again tomorrow.

Cheers and thank you

crssi commented 6 years ago

OK... second round, now with more clear brain :):

73... the URL is still not left intact, but now does not cause breakage

75... didn't test this one

93... OK now

91... OK now and count number is correct

76... didn't test this one

83... Works but not with wildcard

Add #?utm_* Visit http://www.cuisineactuelle.fr/recettes/mini-burgers-au-foie-gras-331523#utm_source=Facebook&utm_medium=social&utm_campaign=PassionApero

and I am puzzled why the question mark is needed

Thank you

Smile4ever commented 6 years ago

73: If it does not cause breakage, I'm happy enough.

75: @gwarser can you test this?

93: OK

91: OK

76: I tested this, OK.

83: Should work now, please retest. Fixed by https://github.com/Smile4ever/firefoxaddons/commit/36768eebebae480bff86a24188a258a665960fa7

and I am puzzled why the question mark is needed

Exact hash parameters need to use #, for example #bio https://archive.is/http://www.mtv.com/music/artist/franz_ferdinand/artist.jhtml#bio

Garbage like #?utm_source=hi&utm_campaign=github needs to use #? https://vivaldi.com/blog/teamblog/vivaldi-1-13-adds-window-panel/#pk_campaign=newsletter&pk_kwd=community

This is because those hash parameters behave like search parameters, but from a technical standpoint they are not. I could support #& as well, but I think it's better to only have one variation so users don't get confused more: #?

So to answer your question: when garbage (and most of the time, but optionally &) comes after #, and you want to delete that parameter, use #?

Edit: To be clear: the ZIP file has been updated again so if you want to test #83 again you will be able to in an easy way. Thanks for your help so far!

gwarser commented 6 years ago

75 should be fixed after https://github.com/Smile4ever/firefoxaddons/pull/82

76 switching from my local tld (.pl) to .com is working, but I cannot switch to other tld - for example .co.uk. I was thinking to rather remove gws_rd from rules lists - not to patch it in code.

crssi commented 6 years ago

83 works now also with wildcards. Thank you :)

Just one more thing: If there is CR (or NL - new line) in between or at the end of the blocked parameters then everything stops working. Maybe the solution would be to replace every CR and/or NL with "," and then trim all by "," to get rid of possible "," as prefix and suffix would solve the confusion. ;)

Otherwise perfect job. Thank you @Smile4ever. :)

crssi commented 6 years ago

Do you have any ETA for publishing it to AMO?

Smile4ever commented 6 years ago

Tomorrow evening probably.

Smile4ever commented 6 years ago

I found another small bug, I've worked around it. https://github.com/Smile4ever/firefoxaddons/commit/f78dd77bff671ebe295f9d0e02c5dda34367d8b3

And as requested, newline removal: https://github.com/Smile4ever/firefoxaddons/commit/99cf8d735fbdd08a99621dd164b0defe4e5390c6

4.0.0 is now available on addons.mozilla.org.

Smile4ever / firefoxaddons

Neat URL - Testing for 4.0.0 #94

Task list for Smile4ever

73... the URL is still not left intact, but now does not cause breakage

75... didn't test this one

93... OK now

91... OK now and count number is correct

76... didn't test this one

83... Works but not with wildcard

73: If it does not cause breakage, I'm happy enough.

75: @gwarser can you test this?

93: OK

91: OK

76: I tested this, OK.

83: Should work now, please retest. Fixed by https://github.com/Smile4ever/firefoxaddons/commit/36768eebebae480bff86a24188a258a665960fa7

75 should be fixed after https://github.com/Smile4ever/firefoxaddons/pull/82

76 switching from my local tld (.pl) to .com is working, but I cannot switch to other tld - for example .co.uk. I was thinking to rather remove gws_rd from rules lists - not to patch it in code.

83 works now also with wildcards. Thank you :)