Closed Bug 1220142 Opened 9 years ago Closed 8 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect)

35 Branch
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: Margaret, Unassigned)

References

()

Details

Attachments

(3 files)

Attached image 2015-10-30 13.05.23.png
There is no context menu available on these images.
What does the DOM look like?
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
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
Yeah, I can't reproduce this anymore.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: