Open Bug 1125308 Opened 9 years ago Updated 2 years ago

Apply pseudo-class to image elements with known size blocked with error NS_ERROR_TRACKING_URI

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
macOS
Type:
defect

Tracking

()

People

(Reporter: javaun, Unassigned)

References

Details

We are testing Tracking Protection, a feature which blocks tracking loads at the network layer. Often these trackers are scripts that do not have a visible manifestation, such as analytics. Sometimes these trackers do have a visible manifestation, such as an ad unit or social button. 

Some test users who have activated the feature are puzzled when they see blank spaces on the page and don't understand that this is where Tracking Protection has blocked a tracker. 

One possible remediation is a that a grey overlay box labeled "Content Blocked for containing Trackers) appears in the area where a formerly visible element was removed. These grey areas would not have to be clickable. 

Examples of how the Tracking Protection feature looks now:
Before: http://imgur.com/2NRUShV
After: http://imgur.com/bWBcoS2

More on TP here: https://intranet.mozilla.org/TrackingProtection#How_does_it_work.3F

Requirement: 
* Investigate feasibility to insert a visible placeholder, such as an overlay box, in place where visible DOM elements are blocked.
* If it's easier to add visible placeholders for all elements (including invisible trackers like analytics scripts that do not render visibly on the page) that's fine too
* Deliver a rough ballpark estimate of effort to solve it.
So what kinds of content do we block and how? Any pointers to the code implementing this all?
(In reply to Olli Pettay [:smaug] from comment #1)
> So what kinds of content do we block and how? Any pointers to the code
> implementing this all?

https://mxr.mozilla.org/mozilla-central/search?string=NS_ERROR_TRACKING_URI

Scripts, images, CSS, object loader. Enforcement happens in necko in nsChannelClassifier. If the URL of the object being loaded is blocked directly then we have a chance. If the object is not blocked directly but depends on another resource that's being blocked, then I don't think we can do anything.

Incidentally, I filed a similar bug https://bugzilla.mozilla.org/show_bug.cgi?id=1063836
specifically for images and the decision was that modifying the broken image rendering was out of scope because it was against the spec.
(In reply to [:mmc] Monica Chew (please use needinfo) from comment #2)
> If the object is not blocked directly but
> depends on another resource that's being blocked, then I don't think we can
> do anything.

https://bugzilla.mozilla.org/show_bug.cgi?id=1120020 is an example of this.
Also I should mention that HTMLDocument.blockedTrackingNodes is an easy way to access the list of blocked nodes.

http://mxr.mozilla.org/mozilla-central/source/dom/webidl/HTMLDocument.webidl#101
Here's a summary of the types of elements that are being blocked on the Alexa top 200 news sites.

Tracking Element type
SCRIPT: 1,890 (78.8%)
IMG: 347 (14.5%)
IFRAME: 149 (6.2%)
OBJECT: 13 (0.5%)
This might be mostly a layout thing, depending on how this should be fixed.
Let's cast aside drawing a grey overlay onto blocked elements and simplify this. The problem we’re really trying to solve is: can we give users contextual feedback whenever we believe that a page load block is likely to visibly affect the page layout users see? 

Monica suggests that images are the easiest low-hanging fruit, but that some blocked images are 1x1, some invisible, and some have no dimensions and hard to predict how they will appear until we render the page. 

Here's an example "good enough" heuristic to signal a high potential for visible impact to page layout.

* Images with *at least* one given dimension — width or height --  and greater than 10 pixels ( or pick an integer  > 1 ) 

Can platform send the UI a message if images meeting this criteria are blocked by TPS? Ideally, we'd want full data on blocked elements (not just a binary signal) so that front end can either adjust the single notification threshold or trigger different notifications at different thresholds. You might send us an object containing 3 blocked elements (for illustration only):

* blocked image, 300w
* blocked image, 200w x 200h
* blocked image, 450w x 20h

Is this sounding more feasible?
Flags: needinfo?(bugs)
(In reply to Javaun Moradi [:javaun] from comment #7)
> Can platform send the UI a message if images meeting this criteria are
> blocked by TPS? Ideally, we'd want full data on blocked elements (not just a
> binary signal) so that front end can either adjust the single notification
> threshold or trigger different notifications at different thresholds.

As I mentioned in comment 4, the HTML document has pointers to a NodeList of blocked tracking nodes, so hopefully that's enough information to introspect on image attributes.
From bz in bug 1081439:

Consider what happens when http://vakin.livejournal.com/780081.html is loaded with tracking protection on.  All the nice bridge pictures just show up as broken image icons, with no indication as to what went wrong.

I propose we do the following things:

1)  When something is blocked by tracking protection, consider reporting it to the console.

2)  For images in particular, consider using a particular nsresult code for canceling them or something, so we can make them match a particular pseudo-class that we then style to provide more information to the user (akin to what we do with blocked plug-ins).
(In reply to [:mmc] Monica Chew (please use needinfo) from comment #10)
> From bz in bug 1081439:
> 
> Consider what happens when http://vakin.livejournal.com/780081.html is
> loaded with tracking protection on.  All the nice bridge pictures just show
> up as broken image icons, with no indication as to what went wrong.
> 
> I propose we do the following things:
> 
> 1)  When something is blocked by tracking protection, consider reporting it
> to the console.

We do this in the security warnings since bug 1079338.
 
> 2)  For images in particular, consider using a particular nsresult code for
> canceling them or something, so we can make them match a particular
> pseudo-class that we then style to provide more information to the user
> (akin to what we do with blocked plug-ins).

We cancel loads with NS_ERROR_TRACKING_URI. For images this happens in http://mxr.mozilla.org/mozilla-central/source/dom/base/nsImageLoadingContent.cpp#175


171       /* Handle image not loading error because source was a tracking URL.
172        * We make a note of this image node by including it in a dedicated
173        * array of blocked tracking nodes under its parent document.
174        */
175       if (errorCode == NS_ERROR_TRACKING_URI) {
176         nsCOMPtr<nsIContent> thisNode
177           = do_QueryInterface(static_cast<nsIImageLoadingContent*>(this));
178 
179         nsIDocument *doc = GetOurOwnerDoc();
180         doc->AddBlockedTrackingNode(thisNode);
181       }
1) sounds good. And don't we do that already. At least I'm seeing 
stuff like The resource at "http://www.google-analytics.com/ga.js" was blocked because tracking protection is enabled. 
all the time after I enabled the pref.

2) So we'd handle
https://mxr.mozilla.org/mozilla-central/source/dom/base/nsImageLoadingContent.cpp?rev=457883ac8776#175
in a bit differently.
Don't show the broken image, but some other image, alt text + some explanation?
That is handled in layout http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsImageFrame.cpp

I might not be the best person to answer to the layout questions. Monica, you may want to ask dbaron
(aren't you even in the same office ;) )
Flags: needinfo?(bugs) → needinfo?(dbaron)
Ha, yeah -- I know dbaron and seth (imagelib owner). I'll ask next time I see them, but isn't the PM's role to find owners for blocking bugs? :) I am but a lowly IC.
Just to point out a similar scenario and solution, when there's an in-content plugin that hasn't been loaded due to click-to-play, a placeholder element is inserted with some css and maybe some xul bindings: https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/shared/plugins/pluginProblem.css
(In reply to David Keeler [:keeler] (use needinfo?) from comment #14)
> Just to point out a similar scenario and solution, when there's an
> in-content plugin that hasn't been loaded due to click-to-play, a
> placeholder element is inserted with some css and maybe some xul bindings:
> https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/shared/plugins/
> pluginProblem.css

Yep, that works if we block the object directly, which looks like it might be happening only 0.5% of the time from comment 5.

Seth, what are your thoughts on comment 12? We discussed something similar in bug 1063836, and you rightly pointed out it was in violation of the image spec.
Flags: needinfo?(seth)
(In reply to [:mmc] Monica Chew (please use needinfo) from comment #13)
> Ha, yeah -- I know dbaron and seth (imagelib owner). I'll ask next time I
> see them, but isn't the PM's role to find owners for blocking bugs? :) I am
> but a lowly IC.
I'd say it is also about the module owner of the layout to figure out how to fix a bug which needs changes to the layout code. Or if the module owner doesn't have the time, he or she can ask other peers of that code to take a look. dbaron happens to be the module owner ;)

https://wiki.mozilla.org/Modules/Core#Layout_Engine
> We do this in the security warnings since bug 1079338.

Ah, excellent.  To point out the obvious, that was fixed after the build I filed bug 1081439 with was created.  ;)

As far as the layout end of things, hooking up some styling based on that nsresult would not be hard, esp. if we don't care whether the web page detecting that the image was blocked by tracking protection.  What we then do with the styling is up for grabs, of course.
(In reply to [:mmc] Monica Chew (please use needinfo) from comment #15)
> Seth, what are your thoughts on comment 12? We discussed something similar
> in bug 1063836, and you rightly pointed out it was in violation of the image
> spec.

It does seem like we need to do something here, I agree. In that bug we were discussing just making the alt text empty in this situation, which I wasn't sure was the best path. But it sounds like it's going to lead to user confusion if we don't offer some sort of visible feedback in this situation, and I think handling this in a similar way to how we handle blocked plugins makes sense.

It's unfortunate that we don't necessarily know the size of the image, though. We'll have to choose the size of the placeholder that we present carefully to minimize layout breakage. (Though not loading the images will inevitably break some layouts anyway, so we can't hope to achieve perfection here.)
Flags: needinfo?(seth)
(In reply to Seth Fowler [:seth] from comment #18)
> It's unfortunate that we don't necessarily know the size of the image,
> though. We'll have to choose the size of the placeholder that we present
> carefully to minimize layout breakage. (Though not loading the images will
> inevitably break some layouts anyway, so we can't hope to achieve perfection
> here.)

I suggest limiting these changes to images for which we have size information as a first step.
(In reply to [:mmc] Monica Chew (please use needinfo) from comment #19)
> I suggest limiting these changes to images for which we have size
> information as a first step.

Yeah, that's a good plan. Definitely avoids any problems with layout.
Note that nothing stops us from creating an nsImageFrame for images that are blocked by tracking protection.  That will still break layout for cases that don't specify a size for the image in the HTML or CSS, but will fix the other cases, right?
Not sure what the needinfo to me is still asking (now that I'm recovered from travel).  comment 10 seems nice and doable.  Providing decent visual feedback for images that have sizes seems doable with that.  Visual feedback for anything else sounds like a research problem.

Happy to answer specific questions, though.  (Modulo time zones and being in standards meetings for the next 5 days.)
Flags: needinfo?(dbaron)
All right then, I'm going to consider that consensus. Javaun, I think it's on you to find an owner for this.
Flags: needinfo?(jmoradi)
Summary: Investigate feasibility of greyed out layout areas when Tracking Protection blocks visible elements → Apply pseudo-class to image elements with known size blocked with error NS_ERROR_TRACKING_URI
That's consensus! By "owner" do you mean someone to champion it for desktop? I can be that person. This bug was just for investigation.

For Monica's recommendation in comment 10 (confirmed by David in comment 22) is this ballpark weeks worth of work, or ballpark months for platform work only?
Flags: needinfo?(jmoradi)
(In reply to Javaun Moradi [:javaun] from comment #24)
> That's consensus! By "owner" do you mean someone to champion it for desktop?

No, I mean find someone to implement it in the timeframe that you deem necessary for the tracking protection work. You did say 38, right? Of course, if this is a non-blocking bug, then that becomes significantly easier :)

> I can be that person. This bug was just for investigation.
> 
> For Monica's recommendation in comment 10 (confirmed by David in comment 22)
> is this ballpark weeks worth of work, or ballpark months for platform work
> only?
Hi David,

I've copied and further specified Monica's requirements from comment 10. Can you give us a rough ballpark (i.e. weeks vs. Months) for:  
* Just items 1a and 1b below
* All of 1a, 1b, 2 

(In reply to [:mmc] Monica Chew (please use needinfo) from comment #10)
> From bz in bug 1081439:
> 
> Consider what happens when http://vakin.livejournal.com/780081.html is
> loaded with tracking protection on.  All the nice bridge pictures just show
> up as broken image icons, with no indication as to what went wrong.
> 
> I propose we do the following things:
> 
> 1a)  When something is blocked by tracking protection, consider reporting it
> to the console. 

1b) (NEW requirement) When images are blocked (as in 1a above), report their width/height attributes when known, even if just a single attribute (w or h) is known. 
> 
> 2)  For images WHERE BOTH W AND H ARE KNOWN (< Javaun's Edits) in particular, consider using a particular nsresult code for
> canceling them or something, so we can make them match a particular
> pseudo-class that we then style to provide more information to the user
> (akin to what we do with blocked plug-ins).
Flags: needinfo?(dbaron)
> 1a)  When something is blocked by tracking protection, consider reporting it
> to the console. 

This work is already done, as I mentioned in comment 11.
I see David's on vacation until 3/2. I'll clear this and run it down next week. Thanks
Flags: needinfo?(dbaron)
Component: DOM → DOM: Core & HTML
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.