Page MenuHomePhabricator

[For visibility] Page previews should be externally extendable
Closed, ResolvedPublic1 Estimated Story Points

Description

Currently the only way to add support for a custom type is to add code to the Popups extension (e.g. https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Popups/+/527547/).

Page previews should provide a generic API and a way to register additional code (e.g. https://www.mediawiki.org/wiki/Manual:Extension_registration#Attributes) so that extensions can host their own code.

QA steps

  1. Visit https://en.wikipedia.beta.wmflabs.org/wiki/Dog and hover over links. Preview should display (don't worry if the preview itself says "There was an issue..."
  2. Hover over a reference link e.g. [8] and check a preview shows with information on reference.
  3. Click the cog on one of the previews and disable "Enable reference previews" or "Enable previews". both previews should no longer show.

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

@Physikerwelt I understand your frustration. Writing on a Saturday as a volunteer, I am also frustrated that I've not been able to find a way forward for you after so long. I think the challenge here is there are not enough design resources or engineering resources to deeply think about and architect the code required to support this use case from the WMF or WMDE side. In short, this is not going to happen without a push from your side.

I want to help you make that happen, and help you in a way that reduces the involvement of all the people that have so far been tagged. While I can't write the patches needed to resolve this task and your larger Math task, I can certainly guide you better and provide code review assistance.

From an engineering perspective, Andreg-p's patch changed how Popups worked in a significant way which made me and possibly others uncomfortable. Previously all previews were trigged by anchor tags (A) and the patch changed that to also apply to elements that were not anchor tags ('#mw-content-text .mwe-math-element[data-qid]'. It also adds an API that I am not familiar with.

I would not be comfortable maintaining such code, so for me a requirement is that this code lives inside the Math extensions.

With that in mind, In terms of the patch at https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Popups/+/527547/2/src/index.js my suggestion of actionable steps here to get this done are as follows:

Step 1

Move all the code in Andre's patch to a Gerrit patch for the Math extension. To get you started, the Math extension should be responsible for wiring up the preview and triggering the preview like so:

popups.registerPreview( '#mw-content-text .mwe-math-element[data-qid]', function (ev, popupOptions) {
                            popups.requestPreview( '#math_<qid>' /* something unique for each math preview */, 'MATH' /*type */, gateway, popupOptions ); } );
    } )
  • The registerPreview or requestPreview methods obviously don't exist... yet.

I would note, this feature should have a feature flag so that it can be enabled/disabled easily in production wikis. We've done this for all the previews so far, and it's important given the scale of our projects that we have a switch we can use to turn it off in cases of API volume problems.

Step 2

To check the Math extension is wired up correctly, let's get ext.popups.main exporting a module.

module.exports = {
  registerPreview: function () { alert('Preview registered'); },
  requestPreview(: function () { alert('Click handler working'); }
};

Step 3

is the bit I'd come to help you with this. What we're looking for is the smallest patch I can help merge to possibly unblock you. You'll want to refactor the existing code to provide those wrapper functions above:

function  registerPreview( selector, handler ) {
             $( document )
                    .on( 'mouseover keyup', selector, function ( event ) {
                      ....
                       handler( event, {
                measures, generateToken, event
            } );
.                     .... on( 'mouseout blur
}

registerPreview( validLinkSelector, function ( ev, popupOptions ) {
            const mwTitle = titleFromElement( this, mw.config );
            if ( !mwTitle ) {
                return;
            }
            const type = getPreviewType( this, mw.config, mwTitle );
            let gateway;

            switch ( type ) {
                case previewTypes.TYPE_PAGE:
                    gateway = pagePreviewGateway;
                    break;
                case previewTypes.TYPE_REFERENCE:
                    gateway = referenceGateway;
                    break;
                default:
                    return;
            }
            requestPreview( title, type, gateway, popupOptions );
} );

module.exports = { registerPreview,  requestPreview };
// (or window.Popups if necessary)

Hopefully, at the end of this we'll have a patch that works as Andre implemented, that my team is happy to support.

Does that make sense? Any questions? Does that seem achievable?

@Jdlrobson thank you very much.

I think this is a great approach. However, the API between step 1/3 is not entirely clear to me. So step 3 would be code that lives inside the popups extension? Maybe we can clarify the API before we start with the implementation? So there might be an intermediate version between step 2/3 that just checked if the API is implemented correctly?

Moreover, I think before we start to introduce more JS code in the math extension codebase, we would want to setup https://www.mediawiki.org/wiki/Manual:JavaScript_unit_testing , would you agree?

Note, that there is also an a-ement for math, e.g., https://en.wikipedia.org/wiki/Schr%C3%B6dinger_equation

<span class="mwe-math-element" data-qid="Q165498">
<a href="/w/index.php?title=Special:MathWikibase&amp;qid=Q165498" style="color:inherit;"><span class="mwe-math-mathml-inline mwe-math-mathml-a11y" style="display: none;"><math xmlns="http://www.w3.org/1998/Math/MathML" alttext="{\displaystyle i\hbar {\frac {d}{dt}}\vert \Psi (t)\rangle ={\hat {H}}\vert \Psi (t)\rangle }">
  <semantics>
    <mrow class="MJX-TeXAtom-ORD">
      <mstyle scriptlevel="0" displaystyle="true">
        <mi>i</mi>
        <mi class="MJX-variant">ℏ<!-- ℏ --></mi>
        <mrow class="MJX-TeXAtom-ORD">
          <mfrac>
            <mi>d</mi>
            <mrow>
              <mi>d</mi>
              <mi>t</mi>
            </mrow>
          </mfrac>
        </mrow>
        <mo fence="false" stretchy="false">|</mo>
        <mi mathvariant="normal">Ψ<!-- Ψ --></mi>
        <mo stretchy="false">(</mo>
        <mi>t</mi>
        <mo stretchy="false">)</mo>
        <mo fence="false" stretchy="false">⟩<!-- ⟩ --></mo>
        <mo>=</mo>
        <mrow class="MJX-TeXAtom-ORD">
          <mrow class="MJX-TeXAtom-ORD">
            <mover>
              <mi>H</mi>
              <mo stretchy="false">^<!-- ^ --></mo>
            </mover>
          </mrow>
        </mrow>
        <mo fence="false" stretchy="false">|</mo>
        <mi mathvariant="normal">Ψ<!-- Ψ --></mi>
        <mo stretchy="false">(</mo>
        <mi>t</mi>
        <mo stretchy="false">)</mo>
        <mo fence="false" stretchy="false">⟩<!-- ⟩ --></mo>
      </mstyle>
    </mrow>
    <annotation encoding="application/x-tex">{\displaystyle i\hbar {\frac {d}{dt}}\vert \Psi (t)\rangle ={\hat {H}}\vert \Psi (t)\rangle }</annotation>
  </semantics>
</math></span><img src="https://wikimedia.org/api/rest_v1/media/math/render/svg/4e59451da64dff4be22db56d1adf9968e2a32d9c" class="mwe-math-fallback-image-inline" aria-hidden="true" style="vertical-align: -2.005ex; width:22.18ex; height:5.509ex;" alt="{\displaystyle i\hbar {\frac {d}{dt}}\vert \Psi (t)\rangle ={\hat {H}}\vert \Psi (t)\rangle }"></a></span>

so modifying the special page is also an option... maybe it can return a title element? I am just thinking about this with the hope to find a way that math is not more special than anything else, which will simplify maintenance in the future.

I think this is a great approach. However, the API between step 1/3 is not entirely clear to me. So step 3 would be code that lives inside the popups extension? Maybe we can clarify the API before we start with the implementation? So there might be an intermediate version between step 2/3 that just checked if the API is implemented correctly?

Yep it would be a bit chicken / egg in that way. We'd want to define a public function in page previews that supports extension for the Math use case. It would be informed by step 1. My expectation right now is that for page previews to be extendable it MUST export a function that allows registering a page previews, and it MAY export a function that allows rendering a page preview related to a certain element (to support use cases different from the current one e.g. that might be showing a page preview in somewhere we currently don't support e.g. special pages).

Moreover, I think before we start to introduce more JS code in the math extension codebase, we would want to setup https://www.mediawiki.org/wiki/Manual:JavaScript_unit_testing , would you agree?

I think unit testing in the Math codebase would definitely be a good idea to help you (and us) maintain the solution. Presumably any test in the Math extension should be run alongside Popups Gerrit patches to make sure nothing gets broken.

Note, that there is also an a-ement for math, e.g., https://en.wikipedia.org/wiki/Schr%C3%B6dinger_equation

Right. The reason I suggested a render page preview function is because of current expectations around links. For each eligible link https://github.com/wikimedia/mediawiki-extensions-Popups/blob/master/src/index.js#L201 we obtain a title that is used to define the page preview type. The current URL is not eligible in that sense because it does contain a hash fragment or point to a page that is valid for a preview. We'd need to think this through during step 3. I think trying to define the simplest API for extension is the problem we're trying to solve to get to this end result, and why this ticket stalled like it did.

@Jdlrobson I have implemented a first rough draft along the lines of your proposal from above:

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Math/+/710329

I have some questions:

  1. Is there an interface for a gateway class that one can implement?

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Math/+/710329/7/modules/ext.math.popup.js#50

In terms of testing, it would be good to make sure that the gateway-triple matches its desired signature.

  1. I have been using the new MW REST API which is almost identical to the RESTbase API. I am expecting config.endpoint to be the location of the rest.php endpoint in that particular wiki. I have no idea how this can be implemented.
  1. I realized that there is a UI bottom to disable popups (I think if you click on the settings wheel of the page preview popup). I think we need a hook to pass the message strings of the name of the math extension popup.
  1. Regarding step two: I do not understand how module.exports works outside node. I (ab)used the mw global instead.

... I have the impression we are at the beginning of a interface definition, and more questions will occur during the process. However, we now have something to start with.

I'm sorry @Physikerwelt I missed this last message because of a vacation. Will get back to you shortly. Sorry for the delay here.

Is there an interface for a gateway class that one can implement?

There's no interface right now.

In terms of testing, it would be good to make sure that the gateway-triple matches its desired signature.

We could realize this by creating a test in Special:JavaScript/qunit that tests the integration between page previews and Math extension. If this test fails we could block Page previews merges. That should get the desired result.

I have been using the new MW REST API which is almost identical to the RESTbase API. I am expecting config.endpoint to be the location of the rest.php endpoint in that particular wiki. I have no idea how this can be implemented.

I'm not too familiar with this. Hopefully @Pchelolo will be able to help?

I realized that there is a UI bottom to disable popups (I think if you click on the settings wheel of the page preview popup). I think we need a hook to pass the message strings of the name of the math extension popup.

Just to check I understand, you want to make it possible to disable Math previews in this dialog:

Screen Shot 2021-09-08 at 9.17.49 AM.png (302×786 px, 35 KB)

I think that's possible, but I'd suggest we do that in a second phase and focus on getting the minimum viable product deployed. For now I think it's fine to tie whether Math popups are on/off to the existing checkbox.

Regarding step two: I do not understand how module.exports works outside node. I (ab)used the mw global instead.

Using mw.popups is the correct solution here and will be better supported.

Is there an interface for a gateway class that one can implement?

There's no interface right now.

OK. Then this is done.

In terms of testing, it would be good to make sure that the gateway-triple matches its desired signature.

We could realize this by creating a test in Special:JavaScript/qunit that tests the integration between page previews and Math extension. If this test fails we could block Page previews merges. That should get the desired result.

Okay. Should this be done as a follow-up task or a preparation task?

I have been using the new MW REST API which is almost identical to the RESTbase API. I am expecting config.endpoint to be the location of the rest.php endpoint in that particular wiki. I have no idea how this can be implemented.

I'm not too familiar with this. Hopefully @Pchelolo will be able to help?

Ah I found that for the action api. We can start with that... I'll create a subtask to find a better solution. T290710

I realized that there is a UI bottom to disable popups (I think if you click on the settings wheel of the page preview popup). I think we need a hook to pass the message strings of the name of the math extension popup.

Just to check I understand, you want to make it possible to disable Math previews in this dialog:

Screen Shot 2021-09-08 at 9.17.49 AM.png (302×786 px, 35 KB)

I think that's possible, but I'd suggest we do that in a second phase and focus on getting the minimum viable product deployed. For now I think it's fine to tie whether Math popups are on/off to the existing checkbox.

To which one References or Links?

Regarding step two: I do not understand how module.exports works outside node. I (ab)used the mw global instead.

Using mw.popups is the correct solution here and will be better supported.

Ok this is resolved then too.

Jdlrobson added a subscriber: ovasileva.

@Jdlrobson Sorry for the delay. I finally find time to focus on this issue and have taken over Moritz's progress so far. I am keen to work on this now until its done.

Please have a short look on our progress so far, especially here: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Math/+/710329/10/modules/ext.math.popup.js

I think this should finish up our changes in the Math extension for now and finish suggested Step 1. The API exist and the Math extension is more or less ready to provide the popup extension with all necessary information.
The only issue I see, is the missing gateway you suggested.

popups.registerPreview( '#mw-content-text .mwe-math-element[data-qid]', function (ev, popupOptions) {
                            popups.requestPreview( '#math_<qid>', 'MATH', GATEWAY, popupOptions ); } );
    } )

Could you please explain how I should build this gateway so that we can use it on the popup extension side? If I understand your intention right, this gateway object should be our (in the Math extension) API endpoint, right?
The first time I implemented the gateway in the popup extension, I was using the preview/model.js to create a model. However, building the model is no longer available in the Math extension (since its part of the popup extension). So I am not sure how I should make this gateway so that it is of use for you.

Here is the first code I was using when created the gateway directly in the popup extension. As you can see, the fetchPreviewForTitle is the issue, because it uses convertPageToModel which in turn relies on the preview/model.js in the popup extension.

	function createMathGateway( config ) {
		var gatewayConfig = $.extend( {}, {
			language: config.get( 'wgPageContentLanguage' )
		} );
		return createMathApiGateway( $.ajax, gatewayConfig );
	}

	function createMathApiGateway( ajax, config ) {
		function fetch( qid ) {
			var endpoint = config.endpoint;

			return ajax( {
				url: endpoint + encodeURIComponent( qid ),
				headers: {
					Accept: 'application/json; charset=utf-8"',
					'Accept-Language': config.acceptLanguage
				}
			} );
		}

		function fetchPreviewForTitle( title, el ) {
			var qid = el.getAttribute( 'data-qid' );
			var xhr = fetch( qid );
			return xhr.then( function ( page ) {
				return convertPageToModel( page );
			} ).promise( {
				abort: function () {
					xhr.abort();
				}
			} );
		}

		function convertPageToModel( page ) {
			return createModel(
				page.title,
				page.canonicalurl,
				page.pagelanguagehtmlcode,
				page.pagelanguagedir,
				page.extract,
				page.type,
				page.thumbnail,
				page.pageid
			);
		}

		var newVar = {
			fetch: fetch,
			fetchPreviewForTitle: fetchPreviewForTitle,
			convertPageToModel: convertPageToModel
		};
		return newVar;
	}

I should have some bandwidth next week to boot up my page previews and try and work out what this would look like on the page previews side! Hopefully, then this can start falling into place.

Change 813347 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/Popups@master] [POC] Extensible previews

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

I spent the afternoon looking into this. I'm not 100% happy with the resulting architecture, but it helped me think through a few things and definitely seems feasible. I'd be interested in reviews from @thiemowmde and @phuedx who have the most familiarity with how page previews currently works to help guide us in the right direction there.

A few notes:

  • I underestimated the work here. The resulting architecture this would lead to, seems beneficial, as it cleans up a few things, but likely needs a bit more thought since we're expecting to expose this to gadgets and extensions.
  • Page previews was designed to preview pages. In this use case that concept is being broken somewhat, as Math's equations do not have associated links that you can click through. I'm not sure how to get round that, so would suggest that all your equations link somewhere - even if it's just a wikidata page or a Help page. A hard requirement that I don't see an easy way round without even more significant refactoring is tying the element that generates the page preview with a page title. This is needed to help distinguish different links
  • Math currently uses span elements. That could pose problems in capturing hovers on the right element. I found I couldn't capture hovers on span.mwe-math-element but could on span.mwe-math-element a (since the link fills up the entire span)

Here's a working example:
https://gist.github.com/jdlrobson/d555904f1739561a7e614d882acc0712
{F35315622}

Feel free to experiment with https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Popups/+/813347 on the Math side and determine whether this approach could work out and let me know of any other issues you run into.

Hope this is helpful!

Thank you.

A few notes:

  • Page previews was designed to preview pages. In this use case that concept is being broken somewhat, as Math's equations do not have associated links that you can click through. I'm not sure how to get round that, so would suggest that all your equations link somewhere - even if it's just a wikidata page or a Help page. A hard requirement that I don't see an easy way round without even more significant refactoring is tying the element that generates the page preview with a page title. This is needed to help distinguish different links

We currently link to a special page such as https://en.wikipedia.org/w/index.php?title=Special:MathWikibase&qid=Q35875

  • Math currently uses span elements. That could pose problems in capturing hovers on the right element. I found I couldn't capture hovers on span.mwe-math-element but could on span.mwe-math-element a (since the link fills up the entire span)

It might use span or div elements. See

https://www.mediawiki.org/wiki/Extension:Math/Popups

for a demo.

It depends on the rendering mode inline vs displaystyle. In the future, beginning from next year, when Chrome eventually supports MathML we would hope to be able to show the popup for parts of the equation. That would be mrow elements.

@Jdlrobson I implemented everything on our side and it looks good so far:

Emc-Screenshot.jpeg (720×1 px, 146 KB)
Jacobi-Screenshot.jpeg (720×1 px, 158 KB)

I still need to implement some tests and I haven't been able to push my latest update but I will do that presumably over the weekend.

Now, you tagged your commit https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Popups/+/813347 as POC. What are the chances to get this live and how long do we need to wait for this?

Change 710329 had a related patch set uploaded (by AndreG-P; author: AndreG-P):

[mediawiki/extensions/Math@master] REST API endpoint for popups

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

Nice! Great feature!

Now, you tagged your commit https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Popups/+/813347 as POC. What are the chances to get this live and how long do we need to wait for this?

I'm not sure right now. I would ideally like to talk to @phuedx and @thiemowmde who have a lot of knowledge and understanding of this extension but they are in different teams from me. Hopefully I'll be able to get a clearer picture within the next week. I'll keep you updated!

I think that this is a great start! I think the mw.popups.register function is neato. I am particularly pleased by Popups dogfooding it and how easy it was for @Andreg-p to use it in https://gerrit.wikimedia.org/r/710329.

Now, you tagged your commit https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Popups/+/813347 as POC. What are the chances to get this live and how long do we need to wait for this?

I think we need to make a decision as to whether the user should be able to disable types of previews individually. If we do decide this, then we should consider adding a mechanism for registering types of previews on the server-side as logged-in users disable previews via Special:Preferences. We could use extension attributes as suggested in the task description:

mediawiki/extensions/Math/extension.json
{
  "Popups": {
    "PreviewTypes": {
      "Math": {
        "name": "math",
        "namemsg": "popups-math",
        "descriptionmsg": "popups-math-description",
        "module": "ext.math.popups"
      }
    }
  }
}

Would register a preview type with:

  • An internal name of math
  • An external name defined in the popups-math message
  • An description defined in the popups-math-description message
  • An associated definition (i.e. the mw.popups.register() call) in the ext.math.popups module

Where the external name and description will be shown to the user.

Thank you. I support this.

I think we need to make a decision as to whether the user should be able to disable types of previews individually. If we do decide this, then we should consider adding a mechanism for registering types of previews on the server-side as logged-in users disable previews via Special:Preferences. We could use extension attributes as suggested in the task description:

Just as an alternative, this could also be handled within the math settings section. This would also allow for individual adjustments. For example, the popup on <refs could have different options for formatting the references. It might be a good practice to place this option next to the option if one wants to enable or disable the popup altogether.

@phuedx Considering the popup extension will be opened up for potentially more types of popups, a more selective enable/disable mechanism might be the right choice. But from our Math perspective, it does not make much of a difference.

The suggested extension attributes look also good to me. Can you or @Jdlrobson realize this so that we can bring the math popups to live?

Can you or @Jdlrobson realize this so that we can bring the math popups to live?

Hi! I've just got back from vacation. I should have more availability in the latter half of August but ideally September. Right now, unfortunately wrapping up mediawiki.org/wiki/Desktop_improvements is stealing most of my time! I will circle back when I can. Thank you for your patience!

Thank you. I think sooner is better than later. If we can complete the entire thing it before Sep 11th, that would be awesome. I hope that we will get the basic API part from the Math part https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Math/+/710329 merged soon.

Change 710329 merged by jenkins-bot:

[mediawiki/extensions/Math@master] REST API endpoint for popups

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

I've reached out to @ovasileva today to find some time to focus on this. We're currently wrapping up the desktop improvements project and have a few absences so we'll struggling to find a reviewer to help push this forward. She will chat to engineering directors.

@ovasileva @Jdlrobson, thank you. Is there an update on that? For us, the code already on gerrit https://gerrit.wikimedia.org/r/c/813347 is good enough to unblock our progress. We are also happy to refactor this within a short period if a more mature solution is available.

Change 832710 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/Popups@master] Use new Popups API

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

@Jdlrobson looking at https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Popups/+/813347 gives the impression that tests are missing. Do you need help creating tests?

I am hoping to get time with @phuedx on Friday to finish this up.

Change 832710 abandoned by Jdlrobson:

[mediawiki/extensions/Popups@master] Use new Popups API

Reason:

Squashed into https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Popups/+/813347/8

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

@thiemowmde we (@phuedx and I) are planning to land this patch on 17th October. Would appreciate if you someone else who worked on references previews. In particular, we are a bit curious about the logic for reference links without a hash fragment.

@Physikerwelt hope this timeline works for you. Apologies this is dragging out longer than expected.

After talking to Sam I am going to make a few modifications this week. I was planning to do those today, but due to some personal circumstances am a bit behind. I'm super sorry for the delay here I know this has already taken much longer than necessary. Hoping to take a look tomorrow and appreciate your patience on this matter.

Jdlrobson renamed this task from Page previews should be externally extendable to [Interrupt] Page previews should be externally extendable.Oct 21 2022, 4:46 PM
Jdlrobson assigned this task to phuedx.
Jdlrobson renamed this task from [Interrupt] Page previews should be externally extendable to [For visibility] Page previews should be externally extendable.Oct 21 2022, 4:59 PM
Jdlrobson updated Other Assignee, added: Jdlrobson.
Jdlrobson set the point value for this task to 1.

A brief update:

@Jdlrobson and I have spoken and my concerns have very nearly all been addressed. I want to think through whether there's a way to not expose mw.popups.register() to everyone and only extension authors and I also need to test the patch end-to-end.

I'm not sure that's a good end state anyway. It would be nice to be able to rewrite the popups gadget to use the same core as page previews.

@Jdlrobson I somehow lost the overview here. Who needs to make the next turn? (I hope it is not me;-)

I'm not sure that's a good end state anyway. It would be nice to be able to rewrite the popups gadget to use the same core as page previews.

Neato. However, there are a couple of issues that would stop you from achieving it right away:

  1. Page Previews detects whether the user has that gadget enabled and disables itself – this behaviour can be removed when the gadget is being rewritten
  2. There is not method to deregister or override a registered preview type. The gadget conflicts with Page Previews enough that we added the behaviour described in #1

@Jdlrobson: Is supporting the goal of rewriting the gadget be considered in scope? If so, then we should revisit my comments about restricting access to the mw.popups#register() method.

I'm not sure that's a good end state anyway. It would be nice to be able to rewrite the popups gadget to use the same core as page previews.

Neato. However, there are a couple of issues that would stop you from achieving it right away [snip]

Yes, I made the comment solely because I didn't want to see a technical decision made that made that objective harder. :)

@Jdlrobson: Is supporting the goal of rewriting the gadget be considered in scope? If so, then we should revisit my comments about restricting access to the mw.popups#register() method.

Not right now (for the reasons given) but I've intentionally written the code in such a way that we could expose it at a future date with minimum technical effort (the API is exposed public by default, and then made private by Page previews).

Right now page previews maintainers don't have the bandwidth to support the implications of allowing gadgets access to this API. Supporting gadgets would increase the scope of this work considerably, and it's not clear to me if we did that, whether there would even be adoption from the gadget side. Right now our main priority is supporting the Math extension developers.

@Jdlrobson @phuedx For now, it is not our goal to be able to rewrite the gadget. In fact, we just want to utilize the popups as they are right now and show additional information about a mathematical expression (similar to the previews for links). In the far future, it might be interesting to have more possibilities in order to make finer, task-specific adjustments on the popups but that's all out of scope for now. In fact, the little WIP demo by @Jdlrobson entirely fits our needs already.

Hence, I vote for not overcomplicating things in this early stage. But that's of course my very biased opinion :)

@Physikerwelt I haven't. Thanks for pointing that out. I will check if that change works tomorrow.

@Jdlrobson I updated our patch according to your changes and was able to make it work again (based on your patchset 20).

Unfortunately, I was not able to make it work with your suggestions in: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Math/+/845640
When I use your changes, I got a weird error on initialization:

Exception in module-execute in module ext.math.popup: load.php:1:864
TypeError: mw.Rest is not a constructor
    js jQuery
    runScript http://localhost:8080/w/load.php?lang=en&modules=startup&only=scripts&raw=1&skin=vector:12
    execute http://localhost:8080/w/load.php?lang=en&modules=startup&only=scripts&raw=1&skin=vector:13
    doPropagation http://localhost:8080/w/load.php?lang=en&modules=startup&only=scripts&raw=1&skin=vector:6
    setAndPropagate http://localhost:8080/w/load.php?lang=en&modules=startup&only=scripts&raw=1&skin=vector:7
    implement http://localhost:8080/w/load.php?lang=en&modules=startup&only=scripts&raw=1&skin=vector:18
    <anonymous> jQuery

I am unable to find the cause of this rather weird error. After analyzing your patches, I simply changed the mw.popups.register method to your new model structure. See also: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Math/+/818294/9..10

mw.popups.register( {
	type: previewType,
	selector: '.mwe-math-element[data-qid] img',
	gateway: {
		fetch,
		fetchPreviewForTitle
	}}
);

And this worked.

Your suggested module.exports and changes in extension.json, however, lead to the error above. I have still struggle understanding the entirety of extension.json. but I believe, something is missing here that our extension requires. @Physikerwelt do you maybe know what causes this issue? Is there a different (new way) to register an mw.Rest depenedency maybe?

I suspect that using mw.popups.register, as I do right now, is not really what @Jdlrobson wanted since I directly register our popups and the extension.json still contains the ext.popups.main dependency.

I left you a note on your patch.

@Jdlrobson Thank you very much. That was the missing piece. From our site, everything is running again. Following your changes, it seems we are close to an end on your site too?

Change 813347 merged by jenkins-bot:

[mediawiki/extensions/Popups@master] Extensible previews

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

Test Result - Beta

Status: ✅ PASS
Environment: beta
OS: macOS Ventura
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

Visit https://en.wikipedia.beta.wmflabs.org/wiki/Dog and hover over links. Preview should display (don't worry if the preview itself says "There was an issue..."
Hover over a reference link e.g. [8] and check a preview shows with information on reference.
✅ AC1: Click the cog on one of the previews and disable "Enable reference previews" or "Enable previews". both previews should no longer show.

Screen Recording 2022-12-06 at 7.10.10 PM.mov.gif (746×1 px, 3 MB)

Edtadros added a subscriber: Edtadros.

Test Result - Prod

Status:
Environment: enwiki
OS: macOS Ventura
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

Visit https://en.wikipedia.beta.wmflabs.org/wiki/Dog and hover over links. Preview should display (don't worry if the preview itself says "There was an issue..."
Hover over a reference link e.g. [8] and check a preview shows with information on reference.
❌ AC1: Click the cog on one of the previews and disable "Enable reference previews" or "Enable previews". both previews should no longer show.
The previews appear, but there is only a cog on the page preview, not the reference preview. Additionally, when clicking the cog, it takes me to the preferences not to an overlay/dialog where I can select the option.

@Edtadros it looks like it's working for me.. Perhaps we need a quick 10 minutes together?

Setting work as expected, resolving.