Page MenuHomePhabricator

[Image Browsing] swiping through detail view gives different images from those in carousel
Closed, ResolvedPublic

Description

steps:

  1. go to Olivia Rodrigo article while logged in with carousel beta on https://en.wikipedia.org/wiki/Olivia_Rodrigo
  2. open carousel
  3. while staying in detail view, navigate through the images

expected: the set of images while navigating in detail view is the same as what's in the carousel
actual: some images that show up while navigating in detail view, e.g., Lorde, are not in the carousel. also some images, e.g., the infobox image, still don't show up in this detail view navigation while in MFE but do show up when I use the ?mmvBeta=1 URL param.

here's a screen recording on iOS Safari (current default MFE, not MMV): https://drive.google.com/file/d/1SbR-S6dcWPNWdpVDwrIJMeUbJOlRrhxr/view?usp=drive_link

Event Timeline

@SherryYang-WMF: Assuming this task is about the MediaViewer codebase, hence adding that project tag so other people who don't know or don't care about team tags can also find this task when searching via projects or looking at workboards. Please set appropriate project tags when possible. Thanks!

One reason for the mismatch is that the carousel additionally excludes images based on height/width (100x100). Refer to maybeAddMobileCarousel in Hooks.php

$thumbExtractor = new ThumbExtractor(
	array_keys( $this->config->get( 'MediaViewerExtensions' ) ),
	$this->config->get( 'MediaViewerExcludedImageSelectors' ),
	100,
	100,
	$this->config->get( MainConfigNames::ArticlePath )
);

The carousel is a more limited set of images than <all images in the article>; the size limit is a big differentiator, but there are also a bunch of files excluded for other reasons (e.g. signatures)
IMO, the carousel is not a separate set of imagery (where clicking an image opens a lightbox with only those), but rather just another entrypoint into MMV (which may include more media than exclusively carousel) - much in the same way that clicking an image in a gallery launches full MMV rather than a lightbox of just those images in the carousel.
IMO, the current behavior is the intended/desired one.

@SherryYang-WMF This sounds like preferred behavior based on the engineering assessment. Are you okay with me closing this ticket?

Moving to signoff for visibility, as this is expected to be marked down as "declined" rather than actively being worked on!