Tap-to-view image overlay does not appear on large article images on medium.com

RESOLVED WORKSFORME

Status

()

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

People

(Reporter: Margaret, Unassigned)

Tracking

35 Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments)

(Reporter)

Description

2 years ago
Created attachment 8681238 [details]
2015-10-30 13.05.23.png

There is no context menu available on these images.
What does the DOM look like?
Created attachment 8681574 [details]
dom_node_in_question.html

Just took a quick look at this.

On a fresh install of Fennec WITHOUT ctp images ever clicked, we have the data-src attribute already set with the URL:

> <div data-scroll="native" class="progressiveMedia js-progressiveMedia graf-image is-canvasLoaded is-imageLoaded" data-image-id="1*ZuCTVMw5v8VVQFQEX4KMVA.jpeg" data-width="1644" data-height="1140" data-action="zoom" data-action-value="1*ZuCTVMw5v8VVQFQEX4KMVA.jpeg">
>    <img src="https://cdn-images-1.medium.com/freeze/max/30/1*ZuCTVMw5v8VVQFQEX4KMVA.jpeg?q=20" crossorigin="anonymous" class="progressiveMedia-thumbnail js-progressiveMedia-thumbnail">
>    <canvas height="50" width="75" class="progressiveMedia-canvas js-progressiveMedia-canvas"></canvas>
>    <img src="https://cdn-images-1.medium.com/max/600/1*ZuCTVMw5v8VVQFQEX4KMVA.jpeg" class="progressiveMedia-image js-progressiveMedia-image" data-src="https://cdn-images-1.medium.com/max/600/1*ZuCTVMw5v8VVQFQEX4KMVA.jpeg">
>    <noscript class="js-progressiveMedia-inner"><img class="progressiveMedia-noscript js-progressiveMedia-inner" src="https://cdn-images-1.medium.com/max/600/1*ZuCTVMw5v8VVQFQEX4KMVA.jpeg"></noscript>
> </div>

There's something funny happening here right off the bat. Then, when you do enable it, the src attribute is removed instead of being replaced with the placeholder svg.

> <img class="progressiveMedia-image js-progressiveMedia-image" data-src="https://cdn-images-1.medium.com/max/1800/1*ZuCTVMw5v8VVQFQEX4KMVA.jpeg">

How odd..
Keep in mind, we do not use data-src, we use data-ctv-src.
(In reply to Mark Finkle (:mfinkle) from comment #3)
> Keep in mind, we do not use data-src, we use data-ctv-src.

Ah yes, so the bit about the missing src and data-ctv-src is the only relevant bit.
Another look at this and I noticed that Medium does some post-pageload setting of the image. There isn't any img element there initially. From looking at how the page loads, I'm guessing they create the img element when their cover image is done downloading.

So this is probably skipping the shouldLoad call because of that.

Would using a MutationObserver[1] be a good way to observe if new img elements are added or is this an overkill?

[1]: https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver
Flags: needinfo?(mark.finkle)
(In reply to Jonathan Almeida (:jonalmeida) from comment #5)
> Another look at this and I noticed that Medium does some post-pageload
> setting of the image. There isn't any img element there initially. From
> looking at how the page loads, I'm guessing they create the img element when
> their cover image is done downloading.
> 
> So this is probably skipping the shouldLoad call because of that.

I think shouldLoad should be called no matter what. Add some simple high level logging in the shouldLoad call to see if it's being hit at all.
Flags: needinfo?(mark.finkle)
I posted a debug APK build which allows small images for bug 1211296. I'm able to see the blocking image now on this Medium post (and I'm able to unblock it as well).

If the patch fixes that bug, I'll try to see if this is still reproducible once I've pushed that change.

https://bugzilla.mozilla.org/show_bug.cgi?id=1211296#c10
Created attachment 8696790 [details]
Screen Shot 2015-12-08 at 16.29.37.png

I mentioned in bug 1211296 [1] this isn't reproducible anymore since the placeholder image shows up correctly in latest nightly.

Will close this issue soon if this doesn't show up again.


[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1211296#c15
(Reporter)

Comment 9

2 years ago
Yeah, I can't reproduce this anymore.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.