search

xtannier / WebAnnotator

WebAnnotator is a tool for annotating Web pages. WebAnnotator is implemented as a Firefox extension (https://addons.mozilla.org/en-US/firefox/addon/webannotator/), allowing annotation of both offline and inline pages. The HTML rendering is fully preserved and all annotations consist in new HTML spans with specific styles. WebAnnotator provides an easy and general-purpose framework and is made available under CeCILL free license (close to GNU GPL — see the license text), so that use and further contributions are made simple. All parts of an HTML document can be annotated: text, images, videos, tables, menus, etc. The annotations are created by simply selecting a part of the document and clicking on the relevant type and subtypes. The annotated elements are then highlighted in a specific color. Annotation schemas can be defined by the user by creating a simple DTD representing the types and subtypes that must be highlighted. Finally, annotations can be saved (HTML with highlighted parts of documents) or exported (in a machine-readable format).
48 stars 11 forks source link

Sometimes links are not disabled #12

Open kmike opened 10 years ago

kmike commented 10 years ago

Check e.g. https://www.facebook.com/MadHatterCafeWeymouth - for me links are half-enabled on first load (check a link with an address):

<a href="http://bing.com/maps/default.aspx?v=2&amp;pc=FACEBK&amp;mid=8100&amp;where1=119+Main+Street%2C+Weymouth%2C+Massachusetts+02188&amp;FORM=FBKPL0&amp;name=Mad+Hatter+Cafe&amp;mkt=en-US"
 wa_temp_href="http://www.facebook.com/l.php?u=http%3A%2F%2Fbing.com%2Fmaps%2Fdefault.aspx%3Fv%3D2%26pc%3DFACEBK%26mid%3D8100%26where1%3D119%2BMain%2BStreet%252C%2BWeymouth%252C%2BMassachusetts%2B02188%26FORM%3DFBKPL0%26name%3DMad%2BHatter%2BCafe%26mkt%3Den-US&amp;h=WAQF6FsBT&amp;s=1"  
 wa_temp_onclick="LinkshimAsyncLink.swap(this, &quot;http:\/\/www.facebook.com\/l.php?u=http\u00253A\u00252F\u00252Fbing.com\u00252Fmaps\u00252Fdefault.aspx\u00253Fv\u00253D2\u002526pc\u00253DFACEBK\u002526mid\u00253D8100\u002526where1\u00253D119\u00252BMain\u00252BStreet\u0025252C\u00252BWeymouth\u0025252C\u00252BMassachusetts\u00252B02188\u002526FORM\u00253DFBKPL0\u002526name\u00253DMad\u00252BHatter\u00252BCafe\u002526mkt\u00253Den-US&amp;h=WAQF6FsBT&amp;s=1&quot;);" 
 target="_blank" 
 rel="nofollow" 
 onmouseover="LinkshimAsyncLink.swap(this, &quot;http:\/\/bing.com\/maps\/default.aspx?v=2&amp;pc=FACEBK&amp;mid=8100&amp;where1=119+Main+Street\u00252C+Weymouth\u00252C+Massachusetts+02188&amp;FORM=FBKPL0&amp;name=Mad+Hatter+Cafe&amp;mkt=en-US&quot;);">119 Main Street, Weymouth, Massachusetts 02188</a>

Then, after clicking "Activate links", some attributes changes:

<a onclick="LinkshimAsyncLink.swap(this, &quot;http:\/\/www.facebook.com\/l.php?u=http\u00253A\u00252F\u00252Fbing.com\u00252Fmaps\u00252Fdefault.aspx\u00253Fv\u00253D2\u002526pc\u00253DFACEBK\u002526mid\u00253D8100\u002526where1\u00253D119\u00252BMain\u00252BStreet\u0025252C\u00252BWeymouth\u0025252C\u00252BMassachusetts\u00252B02188\u002526FORM\u00253DFBKPL0\u002526name\u00253DMad\u00252BHatter\u00252BCafe\u002526mkt\u00253Den-US&amp;h=WAQF6FsBT&amp;s=1&quot;);" 
 href="http://bing.com/maps/default.aspx?v=2&amp;pc=FACEBK&amp;mid=8100&amp;where1=119+Main+Street%2C+Weymouth%2C+Massachusetts+02188&amp;FORM=FBKPL0&amp;name=Mad+Hatter+Cafe&amp;mkt=en-US" wa_temp_href="http://www.facebook.com/l.php?u=http%3A%2F%2Fbing.com%2Fmaps%2Fdefault.aspx%3Fv%3D2%26pc%3DFACEBK%26mid%3D8100%26where1%3D119%2BMain%2BStreet%252C%2BWeymouth%252C%2BMassachusetts%2B02188%26FORM%3DFBKPL0%26name%3DMad%2BHatter%2BCafe%26mkt%3Den-US&amp;h=WAQF6FsBT&amp;s=1" 
target="_blank" 
rel="nofollow" 
onmouseover="LinkshimAsyncLink.swap(this, &quot;http:\/\/bing.com\/maps\/default.aspx?v=2&amp;pc=FACEBK&amp;mid=8100&amp;where1=119+Main+Street\u00252C+Weymouth\u00252C+Massachusetts+02188&amp;FORM=FBKPL0&amp;name=Mad+Hatter+Cafe&amp;mkt=en-US&quot;);">119 Main Street, Weymouth, Massachusetts 02188</a>

Then, after clicking "Disable links" again, I get the following results:

<a wa_temp_onmouseover="LinkshimAsyncLink.swap(this, &quot;http:\/\/bing.com\/maps\/default.aspx?v=2&amp;pc=FACEBK&amp;mid=8100&amp;where1=119+Main+Street\u00252C+Weymouth\u00252C+Massachusetts+02188&amp;FORM=FBKPL0&amp;name=Mad+Hatter+Cafe&amp;mkt=en-US&quot;);" 
 onclick="LinkshimAsyncLink.swap(this, &quot;http:\/\/www.facebook.com\/l.php?u=http\u00253A\u00252F\u00252Fbing.com\u00252Fmaps\u00252Fdefault.aspx\u00253Fv\u00253D2\u002526pc\u00253DFACEBK\u002526mid\u00253D8100\u002526where1\u00253D119\u00252BMain\u00252BStreet\u0025252C\u00252BWeymouth\u0025252C\u00252BMassachusetts\u00252B02188\u002526FORM\u00253DFBKPL0\u002526name\u00253DMad\u00252BHatter\u00252BCafe\u002526mkt\u00253Den-US&amp;h=WAQF6FsBT&amp;s=1&quot;);" 
 wa_temp_href="http://bing.com/maps/default.aspx?v=2&amp;pc=FACEBK&amp;mid=8100&amp;where1=119+Main+Street%2C+Weymouth%2C+Massachusetts+02188&amp;FORM=FBKPL0&amp;name=Mad+Hatter+Cafe&amp;mkt=en-US" 
target="_blank" 
rel="nofollow">119 Main Street, Weymouth, Massachusetts 02188</a>

On other pages links disabling also was inconsistent. I recall for some pages links were not disabled, but after restarting browser and activating session again they became disabled.

It seems that there is some flaw or a bug in a way links are disabled.

kmike commented 10 years ago

14 fixed an issue with attribute iteration, but there is still an issue with event listeners added using addEventListener. It seems it is not possible to get all listeners using javascript executed in a web page context. But it seems it is possible to do that in extensions using nsIEventListenerService. If it'll work I think we should remove checking for "ajaxify" attributes.

kmike commented 10 years ago

Another option is to deep-clone the page after loading and replace it with a cloned one - event listeners are not cloned when cloneNode is called.

xtannier commented 10 years ago

Even if we can track all listeners, it is probably too costly. Parsing all * elements to get rid of the href and on* attribute is already quite long and the user has time to click on some elements before it's done. Deep cloning the whole page is more elegant if it works. (but "reactivate links" won't work)

kmike commented 10 years ago

I think that deleting event listeners can be much faster than changing elements attributes because attributes are a part of DOM, CSS rules can depend on them and so browser could need to re-render the page - this is not the case for event listeners.

Removing event listeners (either by using nsIEventListenerService or by cloning a page) is more than just deactivating the links - it will stop some drop-down menus from working, and other stuff like that. Are you OK with that? Do we need a separate button/option for it?

xtannier commented 10 years ago

Well my opinion is that the more static the page under annotation, the better. We will be unable to annotation inside a dynamic menu, but I'm not even sure it was possible so far.

kmike commented 10 years ago

Ok, got it. I'll try implementing this soon.

kmike commented 10 years ago

nsIEventListenerService drives me nuts - you can get event listeners, but you can't remove them with removeEventListener. Something with object wrapping seems to be wrong; callback object you get seems to be different from the callback object registered. But maybe I'm doing something wrong. Related ticket: https://bugzilla.mozilla.org/show_bug.cgi?id=890505

Deep-cloning the whole page works fine for removing all events (I did a quick prototype). We should be careful to create popups after the page is cloned, but this approach would work if not being able to reactivate links if fine. I personally don't see much value in reactivating them - why is this feature needed?

kmike commented 10 years ago

test file can be found in my branch: https://github.com/kmike/WebAnnotator/tree/link-disabling2; I haven't commited nsIEventListenerService attempts.

xtannier commented 10 years ago

I think I provided the reactivate feature because it was easy to. We can keep reactivating hrefs and on* attributes and forget about event listeners.