When tap-to-load images are enabled, add option to load all images

RESOLVED FIXED in Firefox 45

Status

()

Firefox for Android
General
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Margaret, Assigned: jonalmeida)

Tracking

35 Branch
Firefox 45
Points:
---

Firefox Tracking Flags

(firefox45 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(4 attachments, 4 obsolete attachments)

(Reporter)

Description

2 years ago
I enabled tap-to-load images on Nightly, and for the most part it's fine to not see images loaded by default, but sometimes there are pages where I just want to load all the images (e.g. an image gallery style article). It would be nice to have a menu item or something somewhere to let me do this. Maybe that could be another image context menu item?
Flags: needinfo?(alam)
(Assignee)

Comment 1

2 years ago
Created attachment 8669519 [details] [diff] [review]
wip_show_all_images_bug_1211295.patch

WIP patch.
Like a "Show All Images" item under the "Show Image"?

That could work. I guess it would just be 1 more item and only for users with this feature turned on.
Flags: needinfo?(alam)
MOAR CONTEXT MENUS!

Actually, no let's not do that. Surely we can be more creative than that.
I wonder if photo galleries are exactly the type of pages that _should_ be blocked. I would suggest using a button toast after showing a single image:

[Image unblocked      | SHOWALL]

If the user picks "SHOWALL" we can unblock all the images.
Comment on attachment 8669519 [details] [diff] [review]
wip_show_all_images_bug_1211295.patch

I want a better UX than another contextmenu
Attachment #8669519 - Flags: review-
(Reporter)

Comment 6

2 years ago
Yeah, I agree another context menu item probably isn't the right solution. I didn't think hard about the UX at all when filing this bug.

I'm trying to think more about the cases where I'd actually want all images to be shown, and sometimes it's because the blocked images cause the website to look all out of whack, so I'd rather just bite the bullet and load them all to see a page that's laid out well.
(Assignee)

Comment 7

2 years ago
(In reply to Mark Finkle (:mfinkle) from comment #4)
> I wonder if photo galleries are exactly the type of pages that _should_ be
> blocked. I would suggest using a button toast after showing a single image:
> 
> [Image unblocked      | SHOWALL]
> 
> If the user picks "SHOWALL" we can unblock all the images.

It might be annoying if you're browsing and you want to selectively unblock images but you keep seeing a toast show up that asking to show all.
Maybe after a user unblocks n images on an individual page we show the toast?

I like the context menu option because it'll only be shown when image blocking is enabled.
(In reply to Jonathan Almeida (:jonalmeida) from comment #7)
> (In reply to Mark Finkle (:mfinkle) from comment #4)
> > I wonder if photo galleries are exactly the type of pages that _should_ be
> > blocked. I would suggest using a button toast after showing a single image:
> > 
> > [Image unblocked      | SHOWALL]
> > 
> > If the user picks "SHOWALL" we can unblock all the images.
> 
> It might be annoying if you're browsing and you want to selectively unblock
> images but you keep seeing a toast show up that asking to show all.
> Maybe after a user unblocks n images on an individual page we show the toast?

I don't think of the toast/snackbar as a follow on for "showing all" which is always shown. I think of it as letting the user know something is happening after their action. Showing/unblocking an image only starts the download process. It could take time for the image to be visible. The toast lets people know the process is started, much like our toasts for "download started" or "printing started". In this case, we also provide the "SHOWALL" action, in case this is the desired action, instead of manually unblocking each image individually.
(Reporter)

Comment 9

2 years ago
Are we sure that all blocked images will be large enough that we're showing a placeholder?

Thinking about this a bit more, another use case for an option to load all images is "this page looks broken, let me load all the images to try to fix things". Obviously we want to avoid busting looking pages as much as possible, but it would be good to give users an escape hatch, similar to "request desktop site". I know I'm an advanced user, but when I ran into bug 1211296, I suspected this setting was causing the problem, so I disabled it in settings, but it would have been nice to make a one-off exception.

So for this case, we would need something like a main menu item, or maybe this is something we could integrate into our "control center", similar to what we do with tracking protection.
(Assignee)

Comment 10

2 years ago
I'll try to make time this weekend, if possible, to prototype both options (the toast and the main menu item).
(Assignee)

Comment 11

2 years ago
NI myself so I don't forget.
Flags: needinfo?(jonalmeida942)
(Assignee)

Comment 12

2 years ago
Didn't get time to do this in the previous weekend. Will have to push this back till I get time since I need to investigate/learn more and that'll take more time.
(Reporter)

Comment 13

2 years ago
Before we spend too much time on this, we should think about whether or not this is something that users actually want. I worry than I'm an unusual case, so I want us to verify my ideas before implementing them :)

Anthony, do you have any ideas here? I think the main reason I would want to add an option to load all images would be a fallback to fix a site that looks too busted when image blocking is enabled.
Flags: needinfo?(alam)
Given the conversation and that we know we're probably not the users that this feature is targeting, I'm glad that we're taking a step back and focusing on the problems here. 

Being less prescriptive about a solution is always helpful. So to summarize:

What're we _really_ trying to solve?

1) offering an "escape hatch" UX when sites break
2) throttle user expectation for this feature

I'm in the same boat of "let's not over-rotate" and I think we should address the major usability issues first. This currently "breaks" some sites because some content on the web out there just isn't prepared for this. Next, we can address user expectations.

(In reply to Mark Finkle (:mfinkle) from comment #8)
> (In reply to Jonathan Almeida (:jonalmeida) from comment #7)
> > (In reply to Mark Finkle (:mfinkle) from comment #4)
> > > I wonder if photo galleries are exactly the type of pages that _should_ be
> > > blocked. I would suggest using a button toast after showing a single image:
> > > 
> > > [Image unblocked      | SHOWALL]
> > > 
> > > If the user picks "SHOWALL" we can unblock all the images.
> > 
> > It might be annoying if you're browsing and you want to selectively unblock
> > images but you keep seeing a toast show up that asking to show all.
> > Maybe after a user unblocks n images on an individual page we show the toast?
> 
> I don't think of the toast/snackbar as a follow on for "showing all" which
> is always shown. I think of it as letting the user know something is
> happening after their action. Showing/unblocking an image only starts the
> download process. It could take time for the image to be visible. The toast
> lets people know the process is started, much like our toasts for "download
> started" or "printing started". In this case, we also provide the "SHOWALL"
> action, in case this is the desired action, instead of manually unblocking
> each image individually.

I like this idea of a toast/snackbar with an option. But, I'm unsure if this is the best way to use it in the long run. It seems like a "Undo" option (i.e. Hide) after the user presses "Show image" would make more sense here.

But do we even know if this is reliable enough as an escape hatch? I've not gotten the breakage that is being discussed but if it's really broken, will the user think to "show image" as the solution?

An alternative is adding a similar toast/snackbar with a "load all" images option on page load. This would trigger at the start of all page loads. If they missed it, they could simply refresh the page. This is more akin to the "Report site issue" super toast too.
Flags: needinfo?(alam)
(Assignee)

Comment 15

2 years ago
(In reply to Anthony Lam (:antlam) from comment #14)
> But do we even know if this is reliable enough as an escape hatch? I've not
> gotten the breakage that is being discussed but if it's really broken, will
> the user think to "show image" as the solution?

If we figure out what the default reaction is for a user when they see a broken page, we can then add a snackbar saying something that says "Page broken? Try show all!".
Flags: needinfo?(jonalmeida942)
(Assignee)

Comment 16

2 years ago
I made a build of the snackbar showing up after an image is unblocked since it was quicker for me. Placing a link here if you want to play around with it as well: fennec.surge.sh/app-showall1-bug-1211295.apk

Will make another one with the snackbar showing up when the page is finished loading.
(Assignee)

Comment 17

2 years ago
Clickable link: http://fennec.surge.sh/app-showall1-bug-1211295.apk
(Assignee)

Comment 18

2 years ago
Created attachment 8682589 [details] [diff] [review]
app-showall1-bug-1211295.patch

Patch for the previous build.
Attachment #8669519 - Attachment is obsolete: true
(Assignee)

Comment 19

2 years ago
Created attachment 8682591 [details] [diff] [review]
app-showall-pageload-bug-1211295.patch

The patch for the snackbar showing up when the page loads, and an apk to test with: http://fennec.surge.sh/app-showall-pageload-bug-1211295.apk
(Assignee)

Comment 20

2 years ago
Created attachment 8682593 [details]
Screenshot.png

Screenshot, because screenshots are fun.
(Assignee)

Comment 21

2 years ago
Created attachment 8682669 [details] [diff] [review]
app-showall-pageload-bug-1211295.patch
Assignee: nobody → jonalmeida942
Attachment #8682591 - Attachment is obsolete: true
Nice work Jon!

I think we should decide on a direction here. From a UX point of view I think the following makes the most sense. So let's go with this to satisfy this bug requirement:

 - Add a toast that offers users to "Show all" images after the "Show image" from the dialog.

In terms of UI and copy, I suggest the following:

+-----------------------------------------+
|                                         |
|  Downloaded image            SHOW ALL   |
|                                         |
+-----------------------------------------+
Flags: needinfo?(jonalmeida942)
(In reply to Anthony Lam (:antlam) from comment #22)
> Nice work Jon!

>  - Add a toast that offers users to "Show all" images after the "Show image"
> from the dialog.

Yeah. This approach seems like the simplest thing to do for now.

> In terms of UI and copy, I suggest the following:
> 
> +-----------------------------------------+
> |                                         |
> |  Downloaded image            SHOW ALL   |
> |                                         |
> +-----------------------------------------+

Might I suggest "Image unblocked"
(Assignee)

Comment 24

2 years ago
Which copy shall I use then?
Flags: needinfo?(jonalmeida942) → needinfo?(alam)
(In reply to Jonathan Almeida (:jonalmeida) from comment #24)
> Which copy shall I use then?

(In reply to Mark Finkle (:mfinkle) from comment #23)
> (In reply to Anthony Lam (:antlam) from comment #22)
> > Nice work Jon!
> 
> >  - Add a toast that offers users to "Show all" images after the "Show image"
> > from the dialog.
> 
> Yeah. This approach seems like the simplest thing to do for now.
> 
> > In terms of UI and copy, I suggest the following:
> > 
> > +-----------------------------------------+
> > |                                         |
> > |  Downloaded image            SHOW ALL   |
> > |                                         |
> > +-----------------------------------------+
> 
> Might I suggest "Image unblocked"

Sure let's go with that.
Flags: needinfo?(alam)
(Assignee)

Comment 26

2 years ago
Created attachment 8684383 [details]
MozReview Request: Bug 1211295 - When tap-to-load images are enabled, add option to load all images r?mfinkle

Bug 1211295 - When tap-to-load images are enabled, add option to load all images r?mfinkle
Attachment #8684383 - Flags: review?(mark.finkle)
(Assignee)

Comment 27

2 years ago
Comment on attachment 8684383 [details]
MozReview Request: Bug 1211295 - When tap-to-load images are enabled, add option to load all images r?mfinkle

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24551/diff/1-2/
(Assignee)

Comment 28

2 years ago
Created attachment 8684411 [details]
Screenshot.png

Updated screenshot of the toast.
Attachment #8682593 - Attachment is obsolete: true
(In reply to Jonathan Almeida (:jonalmeida) from comment #28)
> Created attachment 8684411 [details]
> Screenshot.png
> 
> Updated screenshot of the toast.

nice!
(Assignee)

Comment 30

2 years ago
(In reply to Jonathan Almeida (:jonalmeida) from comment #28)
> Created attachment 8684411 [details]
> Screenshot.png
> 
> Updated screenshot of the toast.

Gah, just noticed the screenshot shows the old copy. The actual text is "Image unblocked" instead of "Downloaded Image".
(Assignee)

Comment 31

2 years ago
This is the try build that I triggered via reviewboard (doesn't show the link here for some reason): https://treeherder.mozilla.org/#/jobs?repo=try&revision=c6997a638661
Comment on attachment 8684383 [details]
MozReview Request: Bug 1211295 - When tap-to-load images are enabled, add option to load all images r?mfinkle

https://reviewboard.mozilla.org/r/24551/#review22127

Can we move this to the place where we handle unblocking a single image?
Attachment #8684383 - Flags: review?(mark.finkle)
(Assignee)

Comment 33

2 years ago
Created attachment 8684490 [details]
Screenshot.png

Updated updated screenshot.
Attachment #8684411 - Attachment is obsolete: true
(Assignee)

Comment 34

2 years ago
Comment on attachment 8684383 [details]
MozReview Request: Bug 1211295 - When tap-to-load images are enabled, add option to load all images r?mfinkle

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24551/diff/2-3/
Attachment #8684383 - Flags: review?(mark.finkle)
Comment on attachment 8684383 [details]
MozReview Request: Bug 1211295 - When tap-to-load images are enabled, add option to load all images r?mfinkle

https://reviewboard.mozilla.org/r/24551/#review22133

Looks good
Attachment #8684383 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 36

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/6e1399c05bd0a982a0a176bff2e0f9180fa767bb
Bug 1211295 - When tap-to-load images are enabled, add option to load all images r=mfinkle

Comment 37

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6e1399c05bd0
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45

Comment 38

2 years ago
Flod linked this bug on the IRC, really wondered about the context the screenshot helped me while translating to Swedish so thank everyone :)
You need to log in before you can comment on or make changes to this bug.