Page MenuHomePhabricator

[Bug] media-list: endpoint contains redirects
Open, NormalPublic

Description

Steps to Reproduce

  1. Open https://en.wikipedia.org/api/rest_v1/page/media-list/Badminton_at_the_1992_Summer_Olympics
  2. Observe "items":[{"title":"File:Olympic_rings.svg","section_id":0,"type":"image","showInGallery":true}]

Expected Results

  • File:Olympic_rings.svg is replaced with File:Olympic_rings_without_rims.svg

Actual Results

  • File:Olympic_rings.svg is there even though it is redirected to File:Olympic_rings_without_rims.svg

Event Timeline

JoeWalsh created this task.Aug 7 2019, 4:29 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 7 2019, 4:29 PM
JoeWalsh triaged this task as Normal priority.Aug 7 2019, 8:00 PM

Change 530003 had a related patch set uploaded (by MSantos; owner: MSantos):
[mediawiki/services/mobileapps@master] WIP: avoid redirects in media titles

https://gerrit.wikimedia.org/r/530003

What specific problem is this causing? We can add an MW API call to the endpoint processing just to resolve redirects, but IMO we need a bit more justification for that.

MSantos claimed this task.Aug 14 2019, 4:30 PM

@Mholloway the specific problem is that File:Olympic_rings.svg isn't what the mobile-html for that page loads. It loads the thumbnails from the redirected image (for example, https://upload.wikimedia.org/wikipedia/commons/thumb/5/5c/Olympic_rings_without_rims.svg/320px-Olympic_rings_without_rims.svg.png). This ticket and T230067 are intertwined. The clients need the exact srcset that mobile-html will attempt to load for that page without having to know the implementation details of mobile-html.

Running locally with the fix for T230067, the srcset isn't listed for File:Olympic_rings.svg at http://localhost:6927/en.wikipedia.org/v1/page/media-list/Badminton_at_the_1992_Summer_Olympics . It looks like this change or some variant of this change is required to complete the fix for T230067

MSantos added a subscriber: bearND.Mon, Aug 26, 4:56 PM

Running locally with the fix for T230067, the srcset isn't listed for File:Olympic_rings.svg at http://localhost:6927/en.wikipedia.org/v1/page/media-list/Badminton_at_the_1992_Summer_Olympics . It looks like this change or some variant of this change is required to complete the fix for T230067

This seems like a different bug. When applying adjustThumbWidths transformation, the srcset is removed because of one conditional in the adjustSrcSet function in the media lib. So, the absence of the srcset is unrelated to redirects.

I don't know the reason behind this logic so I need to ask, @bearND and @Mholloway is there any reason for this logic? (remove the srcset of a thumbnail that failed to adjust)

@MSantos that logic is there so that it doesn't try to request a thumbnail larger than the original file. In this case the data-file-width="342" so all of the srcset sizes are larger than that (ignoring for now that this is a vector image so could actually be scaled beyond this). Either way, the src is present in the resulting page so media-list should be able to pull the src and include it in the media-list srcset.

@JoeWalsh src is present but srcset is removed. If I don't remove srcset the image HTML will be:

<span class="pagelib_theme_image_presumes_white_background pagelib_lazy_load_placeholder pagelib_lazy_load_placeholder_pending" style="width: 320px;" data-class="pagelib_theme_image_presumes_white_background" data-src="//upload.wikimedia.org/wikipedia/commons/thumb/5/5c/Olympic_rings_without_rims.svg/320px-Olympic_rings_without_rims.svg.png" data-srcset="//upload.wikimedia.org/wikipedia/commons/thumb/5/5c/Olympic_rings_without_rims.svg/300px-Olympic_rings_without_rims.svg.png 2x, //upload.wikimedia.org/wikipedia/commons/thumb/5/5c/Olympic_rings_without_rims.svg/225px-Olympic_rings_without_rims.svg.png 1.5x" data-width="320" data-height="147" data-data-file-width="342" data-data-file-height="158"><span style="padding-top: 45.9375%;"></span></span>

That's why I asked about the logic, although now I see it's the scales in the srcset have weird values.

Would it be the case to provide the src value when srcset is not present? I'm thinking something like that:

items: [
    {
        title: "File:Olympic_rings.svg",
        section_id: 0,
        type: "image",
        showInGallery: true,
        srcset: [
            {
                src: //upload.wikimedia.org/wikipedia/commons/thumb/5/5c/Olympic_rings_without_rims.svg/320px-Olympic_rings_without_rims.svg.png,
                scale: "1x"
            }
        ]
    },
]

@MSantos yes - it should use src as 1x when srcset isn't present. I had that as a comment in the JSON example on T230067 but should have made it clearer in the task description.

@MSantos yes - it should use src as 1x when srcset isn't present. I had that as a comment in the JSON example on T230067 but should have made it clearer in the task description.

Reading now I think it was clear, but I missed, sorry about that, a fix is on it's way.

This task is not needed then, or you think there is something else missing?

Change 532719 had a related patch set uploaded (by MSantos; owner: MSantos):
[mediawiki/services/mobileapps@master] media-list: pull src property from image tag

https://gerrit.wikimedia.org/r/532719

Change 532719 merged by jenkins-bot:
[mediawiki/services/mobileapps@master] media-list: pull src property from image tag

https://gerrit.wikimedia.org/r/532719