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)
Tracking
(Not tracked)
RESOLVED
WORKSFORME
People
(Reporter: Margaret, Unassigned)
References
()
Details
Attachments
(3 files)
There is no context menu available on these images.
Comment 1•9 years ago
|
||
What does the DOM look like?
Comment 2•9 years ago
|
||
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..
Comment 3•9 years ago
|
||
Keep in mind, we do not use data-src, we use data-ctv-src.
Comment 4•9 years ago
|
||
(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.
Comment 5•9 years ago
|
||
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)
Comment 6•9 years ago
|
||
(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)
Comment 7•8 years ago
|
||
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
Comment 8•8 years ago
|
||
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•8 years ago
|
||
Yeah, I can't reproduce this anymore.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
Assignee | ||
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•