| Jmabel | |
| Oct 18 2019, 2:08 AM |
| F31062104: Screenshot_2019-11-11_12-05-12.png | |
| Nov 11 2019, 6:28 PM |
| F31062102: Screenshot_2019-11-11_12-05-00.png | |
| Nov 11 2019, 6:28 PM |
| F30838613: Screenshot_2019-10-17 File Fourth Ave looking north from Jefferson St during the Denny Hill regrade, 1909 (SEATTLE 1579).jpg.png | |
| Oct 18 2019, 2:08 AM |
| F30838614: Screenshot_2019-10-17 Preferences - Wikimedia Commons.png | |
| Oct 18 2019, 2:08 AM |
I've been using the "Collapse captions" gadget on Wikimedia Commons for some time. It stopped working (at least for me) a few days ago. The attached files show, respectively, that I have the gadget selected and that when I access a file page, the gadget isn't working.
Works for me in Firefox 69 - I only see the English caption.
@Jmabel: Please follow https://www.mediawiki.org/wiki/Help:Locating_broken_scripts and report back.
If you see "only the English caption" it's probably because you have only a Babel box for English. I have Babel boxes for 8 languages, so I see 8 languages. But with "Collapse captions" selected, we shouldn't be seeing any captions at all, just a thing to click to see captions.
There seems to be a lot of confusion here.
If you see "only the English caption" it's probably because you have only a Babel box for English. I have Babel boxes for 8 languages, so I see 8 languages. But with "Collapse captions" selected, we shouldn't be seeing any captions at all, just a thing to click to see captions.
That's not true. I do not have any babel box in my userpage but I still see captions in languages other than English. Not sure how babel box in userpages affects structured data captions.
This hack generally doesn't seem to work now. I think something has changed in the SDC code that means that the hook no longer works. I'm not sure if that can be fixed within the gadget, or if it requires a tweak to the SDC HTML. Perhaps @Jdforrester-WMF or @Abit could have a look?
So I tried to fix this.
As a 'bug report needs:
Steps to reproduce:
Expected:
Actual:
My findings:
The original code of the gadget is:
$( ".wbmi-entityview-captionsPanel" ).addClass( "mw-collapsible" ); $( ".wbmi-entityview-captionsPanel" ).addClass( "mw-collapsed" );
Under debug mode, The gadget is loaded before wikibase.mediainfo.filePageDisplay module, which destroys the div.wbmi-entityview-captionsPanel at
$( '.wbmi-entityview-captionsPanel' ).replaceWith( captionsPanel.$element );
The DOM is subsequently rebuilt in
var template = mw.template.get( self.moduleName, self.templateName ), $rendered = template.render( data ); self.rebuildDOM( self.$element, $( self.$element.get( 0 ).cloneNode( false ) ).append( $rendered ), preserve ); return self.$element;
This will resolve the captionPanel's renderPromise, so we could just hook its renderPromise, right? Not so simple.
https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/WikibaseMediaInfo/+/c458afeb62a7803c792eac4f75e71048831f6a1f/resources/filepage/init.js#15:
captionPanel is a closure-protected "private variable". Accessing such variables are a PITA.
So, we know rebuildDOM is going to be called, let's try hooking (via monkey patching) on that method (it's not the best idea), in order to get a reference to that variable in a cross-browser way:
mw.loader.using( 'wikibase.mediainfo.base', function () { var ComponentWidget = require( 'wikibase.mediainfo.base' ).ComponentWidget; mw.hook( 'wikipage.content' ).add( function () { $( '.wbmi-entityview-captionsPanel' ).makeCollapsible( { collapsed: true } ); } ); // Ugly hooking :( ComponentWidget.prototype.rebuildDOM = function ( orig ) { return function () { var val = orig.apply( this, arguments ); if ( this.moduleName === 'wikibase.mediainfo.filePageDisplay' ) { this.renderPromise.then( function () { $( '.wbmi-entityview-captionsPanel' ).makeCollapsible( { collapsed: true } ); } ); } return val; }; } ( ComponentWidget.prototype.rebuildDOM ); } );
This workarounded the issue for debug mode, but not non-debug mode. Under non-debug mode, under Firefox 70.0, the monkey patch has been applied when the DOM is rebuilt, but the call stack of the DOM manipulation just does not have our patched method wrapper. I ran out of time to figure out why that is happening, probably needs testing with vagrant if I continue with this.
Also, why can't the DOM elements of OOJS-ui just have an attribute pointing ("expando property") to the instance of OOJS-ui widget? It's not like assigning new attributes to DOM elements will break anything (jQuery does that), or cause leaks (it does cause a cyclic reference, but browsers have cyclic GC), and the runtime won't even see the new attributes (Gecko's X-ray vision). It makes accessing OOJS-ui widget instances a ton lot easier.
The widgets in the WikibaseMediaInfo extension use templates and render() methods to ensure that the visible UI remains in sync with the structured data as it changes. Existing DOM elements are often thrown out and re-rendered from scratch to ensure consistency. Our introduction of this workflow into the Captions widget is probably what initially broke this gadget.
Because the DOM elements are not guaranteed to be persistent here, I'd say that an approach that leans more on CSS overrides is going to be a better bet in the long run. JS can and will continue to change.
This selector will target the individual captions inside the panel: .wbmi-entityview-captionsPanel .wbmi-entityview-captions (you need an extra level of specificity because of some OOUI rules). If the gadget was able to add/remove a class at the top level of .wbmi-tabs-container (the higheset-level DOM element the MediaInfo extension touches), you could override the existing styles with display:none or anything else you wanted. Then you could simply toggle the class on that top-level element (there are no render() calls happening there that would interfere).
.my-toggled-gadget-class .wbmi-entityview-captionsPanel .wbmi-entityview-captions {
display: none;
}Hope this helps.
@zhuyifei1999: Commons is a project tag dedicated to Commons issues and it is set on this task, so this feels like a valid task under Commons.
This is not a good workflow - please reconsider it. It's better to minimize the Javascript as much as possible, and to keep a clean UI in HTML/CSS. Abruptly re-rendering things and throwing things out leads to unexpected behaviour for other developers, like we're seeing here.
Also, please consider bringing this gadget in-house - it was developed because the community asked for it, and at least making the caption box collapsible by default (but still showing it, while giving users an option to auto-collapse it if they want) would be a good step forward.
In the general case, bringing on-wiki matters, such as templates, modules, gadgets, (and, workflows, proposals, etc.), into phab is ineffective. Those who maintain such pages (templates, modules, gadgets) are very unlikely to be watching the entire Commons tag to be notified about the issues at all, it would be likely your (Bugwrangler's) burden to notify the corresponding maintainers of the gadget of the issues with the those pages. In addition, they are often reports from non-technical users who may make reports that are simply not "effective".
Keeping on-wiki matters on wiki, especially under the concerning pages' talk pages, noticeboards, user talk pages, make the issue much more likely to be known. The maintainers are more likely to watch them than a single than a giant group project tag on phab. Exceptions do exist, but they are a minority. In addition, many wiki-editors are more used to using the wiki interface than to the phab interface. Issues will eventually be reported on-wiki because the some reporters prefer one less extra step of learning how to use phab. Tracking an issue on both phab and wiki would be, redundant.
The purpose of these phabricator projects to non-technical users has been a bridge between them and developers. This is in general a bridge between the community and the developers. But the cases where the concerned code is made from within the community... is the bridge really needed?
That said, this task is different. Usually, a script is broken because some HTML class change, module rename, API change, etc, which are usually not too difficult to fix in one edit or two on-wiki. This one is an extension change that completely broke the assumptions the script is running under, and made it extremely difficult, if not impossible, for the script to return to its original behavior. These sort of issues needs attention from extension developers, who are typically more likely to be using phab to track their issues. (Why? Because unlike gadgets / modules / templates, Extensions are privileged (as in, high standards) and often global.) Even if this particular extension in concern is only deployed to Commons, those with a skillset to develop such an extension are still the similar people.
Now that it is asked to move the gadget in-house (extension), that makes this issue no longer an on-wiki matter, but an extension matter. So yes, this task is valid, but it is just an exception in an usually on-wiki matter of gadgets, modules, and templates.
Just to be clear here - we currently have no plans to bring this in-house. Developer time is scarce and there are many things taking higher priority.
Workarounded. https://commons.wikimedia.org/wiki/MediaWiki:Gadget-Collapse-Captions.js Ideally this should still be brought in-house. You now have a MutationObserver, which, just like captionsPanel, is basically "private".
@zhuyifei1999: You may be right, however the scope of Commons is up to discuss with the members of the Commons Phab project... Also see T86444 :)