Repository navigation
Fix tooltips and links in the carousel gallery mode - #134
Merged
Merged
Conversation
malberts
force-pushed
the
fix/gallery-carousel-tooltip-and-link
branch
from
October 29, 2026 09:47
a812ab6 to
8800c61
Compare
Follows-up to #133 `<gallery mode="carousel">` rebuilds each image as `[[File:...]]` wikitext from what core passes to `ImageGalleryBase::add()`, and it mishandled two parts of that data. Core adds a `targetlang` handler option to every gallery image. It is not an image option, so the image parser read `targetlang=en` as the caption, which became every slide's tooltip and the alt text of a captionless slide. And core passes a per-image `link=` in the image options (`link-url`, `link-title` or `no-link`), while the carousel read `add()`'s `$link` argument, which core leaves empty, so every slide linked to its file page. The carousel now drops `targetlang`, which `makeImage` sets again when the slide is parsed, and writes `link=` from the image options. Other handler options, such as `page=` and `lang=`, pass through as before. On MediaWiki 1.47 and later, where `<gallery>` accepts a per-image `class=` and passes it in the image options, that class is added to `img-fluid`. `{{#bootstrap_carousel:}}` is unchanged. `CarouselGalleryTest` fed the gallery a link in `$link`, no `targetlang` and no image options, a shape core never produces, which is how both bugs passed CI. Its data sets now use core's shape, and the new `CarouselGalleryRenderingTest` parses `<gallery mode="carousel">` with the wiki's own parser. Not in this change: a caption containing a link loses the link text in the tooltip, and a caption that is an image option keyword, such as `thumb`, is still read as that option. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
malberts
force-pushed
the
fix/gallery-carousel-tooltip-and-link
branch
from
October 29, 2026 10:25
8800c61 to
a2eb513
Compare
malberts
marked this pull request as ready for review
October 29, 2026 10:41
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows-up to #133
<gallery mode="carousel">rebuilds each image as[[File:...]]wikitext from what core passes toImageGalleryBase::add(), and it mishandled two parts of that data. Core adds atargetlanghandler option to every gallery image. It is not an image option, so the image parser readtargetlang=enas the caption, which became every slide's tooltip and the alt text of a captionless slide. And core passes a per-imagelink=in the image options (link-url,link-titleorno-link), while the carousel readadd()'s$linkargument, which core leaves empty, so every slide linked to its file page. The carousel now dropstargetlang, whichmakeImagesets again when the slide is parsed, and writeslink=from the image options. Other handler options, such aspage=andlang=, pass through as before. On MediaWiki 1.47 and later, where<gallery>accepts a per-imageclass=and passes it in the image options, that class is added toimg-fluid.{{#bootstrap_carousel:}}is unchanged.CarouselGalleryTestfed the gallery a link in$link, notargetlangand no image options, a shape core never produces, which is how both bugs passed CI. Its data sets now use core's shape, and the newCarouselGalleryRenderingTestparses<gallery mode="carousel">with the wiki's own parser.Human input
Goal and shape: Claude's proposal from its review of #133, approved by @malberts: drop
targetlang, honour the link options core passes, and add a test fed by the data core passes. Claude made the technical design within it.Outcome: not reviewed by a human yet. Left for you: the code, and the lines from the evidence table in a
<gallery mode="carousel">on a test wiki, hovering for the tooltip and clicking for the link.Claude's calls
class=is honoured on MediaWiki 1.47 and later.add()'s$linkargument is no longer read. Core's ownTraditionalImageGalleryreads it only when a caller passes no image options, and the core callers that do (category pages and Special:NewFiles with?gallerymode=carousel) already end in an exception in this mode.link=Main Page#a]]b, and they come from the same line of wikitext, so a broken result gives the editor nothing they could not write directly.TraditionalImageGallerydoes, instead of rebuilding wikitext, which would remove this class of bug but is a larger change toCarousel.class=: it would only run on the MediaWiki 1.47 and master CI rows, which are allowed to fail, so aCarouselGalleryTestdata set covers it.page=andlang=: CI imports PNGs only.page=on a PDF.$linkas a fallback for callers that pass no image options.Not in this PR
All predate this PR; the first was hidden behind the
targetlangtooltip until now.File:Serenity.png|See [[Main Page]] nowgives "See now", where a plain<gallery>gives "See Main Page now".File:Shuttle.png|thumbgives a thumbnail with an empty caption and the alt text "thumb", where a plain<gallery>shows "thumb" as the caption.File:Shuttle.png(94px) in a 752px carousel, cannot be hovered or clicked with a mouse; its link still works from the keyboard.{{#bootstrap_carousel:}}behaves the same.Verification evidence
Test wiki: MediaWiki 1.43.11, PHP 8.3.36, Extension:Bootstrap 6.0.0.
src/change, theCarouselGalleryRenderingTesttests and thesimpleandno local attributesdata sets ofCarouselGalleryTest::testToHtmlfail. Without theno-linkbranch,testSlideImageWithEmptyLinkIsNotClickablefails. The new tests run on every CI row.action=parsebefore and after the fix:<gallery mode="carousel">href/title/ imgalt<gallery>File:Serenity.png|Serenity captiontargetlang=en/Serenity captionSerenity caption/Serenity captionFile:Shuttle.pngtargetlang=en/targetlang=enFile:Serenity.png|Internal|link=Main Page/index.php/Main_PageFile:Shuttle.png|External|link=https://example.org/xhttps://example.org/xFile:Serenity.png|Nolink|link=File:Shuttle.png|Fragment|link=Main Page#Crew/index.php/Main_Page#CrewFile:Serenity.png|Interwiki|link=wikipedia:Serenityhttps://en.wikipedia.org/wiki/SerenityFile:Languages.svg|Greeting|lang=delangde-thumbnailFile:Serenity.png|Serenity caption|class=foogives the slide's wrapperclass="mw-default-size foo img-fluid", the same<span>a plain<gallery>putsfooon, andclass=bar baz|link=Main Pagegives both classes and the link. With theCarouselGallery.phpfrom before this PR, both classes are dropped.link=Main Pageandlink=goes to Main Page and stays on the page; thelink=https://example.org/xslide (File:Shuttle.png, under the previous control) goes to example.org from the keyboard.