Closed Ede123 closed 9 years ago
I think there could be the case where several requests and being performed in the background and the "skip" flag might is used for the wrong URL. So, how about if you replace that by appending addon.id
to the URL, then in getLink
you check the URL for the presence of the addon id and if it's found you can then prevent the cleaning.
Actually I started with the approach you describe (I used a randomized "magic" string - randomized to prevent it from being exploited) but found it to be problematic, too:
getLink
and redirect accordingly)http-on-modify-request
observer to clean the link and probably the Redirect Watcher, too.http-on-modify-request
observer I found no practical way to detect whether a link was cleaned on purpose or if it only was cleaned to remove the appended string. This led to the link simply being cleaned twice (once to remove the magic string and a second time after that which results in the wrongly cleaned link again). As long as you don't know an easy way to do it, we'd have to rewrite the observer code and somehow manipulate the HTTP channel to let our observer know we don't want this link to be cleaned. Similar issues probably apply to the Redirect Watcher, too, but I stopped looking into the approach at that point since I considered it to be to complicated with all the additional things it would require...Do you think the problem you described would be a real world issue?
Otherwise I'd still vote for my approach unless we can come up with something more clever.
Hi Eduard, first of all sorry for the quite late reply.
Well, looks like you did your homework ;) so i'll just merge this, i was also concerned about the possibility of the addTab
(and/or related internal logic) being handled on a delayed/async way, in such case the "skip" flag may takes place on the wrong link, however didn't checked/verified if this is a real world issue/scenario, but i guess i'd need to change it for e10s anyway.
Thanks, no problem with the delay (better late then never and I know myself that spare time can be limited).
[Off-topic: Do you already have plans for e10s compatibility?]
Do you already have plans for e10s compatibility?
No plans yet, other than i'll need to get motivated enough and have some time to do it :)
This is particularly useful if a link was cleaned in error and one quickly wants to open the uncleaned link.
There is one difficulty: By default the "http-on-modify-request" observer cleans all opened links, there's no possibility to prevent this (basically that's exactly what we want but obviously it's problematic if we want to open uncleaned links). I addressed this in commit b37666a (which allows links to be skipped from cleaning). I'm not sure if it's the best solution, so if there were better alternatives I'd be open to change the approach.