We discovered a bunch of possible solutions (see previous patch sets),
including replacing the `$( document )` selector with a more specific
one. That idea does not pass the linter.
Very late I realized the original selector starts with
`#mw-content-text`. This heavily limits where popups are allowed to
appear: really only in the main text content area.
We should limit reference popups to the exact same scope.
This fixes the issue described in T215195. Before, the content of the
popup was covered by the selector. Reference links *inside* the popup
would trigger another popup, which makes the current popup disappear.
Now the popup itself is not covered by these event handlers any more.
Bug: T215195
Change-Id: I142aee68abbd57ca321873855fef9209e0db0bbf
Make targets consistent with Mobile Frontend's .babelrc
and put them into a .browserslistrc file.
This did not change the build files.
Change-Id: Ic4fe13c9e4423d4bb5e6da5fce9039d04a9a215f
Add jQuery types. The JSDocs use TypeScript-compatible typing but tools
such as Visual Studio Code and WebStorm requires the definitions be
supplied. Popups will require these definitions to enable any future
automated type checking too.
Change-Id: I27b5cb052c5ad353322181b0f0ffa4fa56ac1d9f
There are a few cheap checks done before this regular expression is
even needed, most notably the check for a pretty URL (without query
parameters). Since the vast majority of links processed by this parser
are pretty, I believe this optimization is worth it.
Change-Id: I730b87dc010161e8bc3f311c517293c0ad553326
So $config is used instead of $conf for consistency and this
is how it's used across many many Wikimedia repos (exts) and
even in core.
Change-Id: Ia8e7335bf6892628e0a8b5d901776d726a8ad743
This is now possible since the render functions return jQuery objects.
All this code is exclusively used in the pagePreview.js file, and
doesn't need to make the already very big renderer.js file even
bigger.
Note the tests for all renderers have always been collected in a
single file. That's why the test case does not move.
Change-Id: I0c24638751c5f0e93d2bc0f3f4bb61fa0cf50d15
This just adds a simple test if passing the id of the clicked
reference source footnote works.
Bug: T213905
Change-Id: Ifc6549aa0203f19a5b24fa854b0aaf0cfb25674d
To make sure that we enable the link highlighting in the Cite extension we want
to trigger the click handler on the original footnote link. This is done by
passing the id of the source element to the model and the renderer.
Bug: T213905
Change-Id: I0bd59ac326269f3c0850946851fb79b611dc2a57
When we started reorganising the code around the gateway selection
we forgot moving the docs and typedefs from the (now) more specific
page.js file to the (now new) index.js. - This patch fixes that.
See also I5efa9fb8f63f1487c627eb9a3f1fe47f43c611cc
Change-Id: I7b3582882247e22497d3a37b896a8a7c9950cf0d
This CSS class is used by the regular wikitext content area, and used
by certain CSS selectors (e.g. the external web link arrow).
The DOM structure for each popup is (intentionally) created outside
of this scope, at the end of the current page's body. This works
great for page previews because the do *not* want to share any styles
with the rest of the page. But reference previews want to do this.
In this patch I also remove the inner <span>. It was misplaced (note
the name) and resulted in block elements nested in an inline element.
Bug: T214463
Change-Id: I740e37a2ed929edf971b348fbf20e5fb12012d37
As discussed, the $( '<a>', { id: 'foo' } ) syntax is bogus because
plugins are able to *change* it. It's not just a list of attributes,
but whenever there is a method with the same name, that method will be
called instead. This means the result of this feature is unpredictable.
This patch also streamlines a few other jQuery calls that can be
shortened.
Bug: T214970
Change-Id: Ib58b8673c7ce41139f926c845c1b3adfbfde1b26
A lead section is essential for a summary in the /restbase/
content service. On commits we test the mediawiki endpoint but
against beta cluster we test the restbase endpoint.
I overlooked this in If855c7c0a2ad65d96d03d6a1411b453ecbe8752b
Bug: T214974
Change-Id: I9959d7ae463c4e1d4fa5345fdb59fe1b2152d49e
The previous Popups test page pointed to the "Main page" which as
of Ie15487184a7f9fc08603fc42cfad3aeac6642dcc has specially handling
This makes a new test page "Popups test page 2" that is linked to from
"Popups test page" which previously linked to the Main page which
now leads to the display of a broken Popup
This gets our test fixed but the problem with main page previews
will remain (T215080)
Bug: T214974
Change-Id: If855c7c0a2ad65d96d03d6a1411b453ecbe8752b
This gets rid of a little bit of code duplication, and makes the
interfaces all conform to one standard again after I05ed4b8 left them
in a little inconsistent (but properly documented) state.
Bug: T214970
Change-Id: If8407c1a48aff1cb31fc2e74b3c2b846e79a3cb5
As discussed in Iaadcce9. This does have a few benefits:
* Less code in the already pretty big render.js file.
* The code setting the target attribute is much closer to where it
belongs: in the file that specifies how the content of a reference
popup should look and behave.
* The class name "mwe-popups-extract" is not mentioned in two different
files, but in the same.
Note this changes the signature of this src/ui/templates/… file to not
return an HTML string any more, but a jQuery object. The other templates
still return strings. I believe this is fine, and not that much of a
difference anyway. The signatures don't need to be identical. And the
jQuery object still represents the exact same HTML as before.
If it helps we could change all templates/… signatures accordingly.
Could be done in this or a separate patch.
Bug: T213908
Bug: T214970
Change-Id: I05ed4b886f79c5ae748f53ab9fed965dfd217620
This solves the (I believe) only regression we introduced: A bad fake
reference like <span class="reference">[[Other article#Section]]</span>
showed a page preview for the "Other article" before we introduced
reference previews, but would have shown nothing after I9ec57e0.
Checking if the link is a self-link solves this and possibly more related
issues. Only self-links can point to a footnote on the same page.
Manually created fake-references like
<span class="reference">[[#Section]]</span> still have a chance to show
nothing in case the manually created HTML does not strictly follow the
expectations in the gateway. There is not much we can do about this. We
should not accept any arbitrary HTML but need to make at least *some*
assumptions.
Bug: T214970
Change-Id: I86e91bf45c3ae4c6a4086f7f1c7b1280fd400d17
We updated this documentation just recently via Ie370cfe. We followed
what the createModel() function does. But this is not the only way a
PreviewModel object can be created. Reference previews, for example,
don't use it.
Instead of following createModel(), I checked what the different popup
types actually use.
Bug: T214970
Change-Id: I2c4293a48387836dc30e18d10d952b4a26e6f2b5
I do find these very confusing and would like to remove them:
* The test setup looks like these popup types are going to use
these properties. But they don't. They are not even trying to
access these properties.
* There are no assertions that make sure these properties are
*not* used. It would be possible to add something like this,
but I honestly think this is not worth it.
We might need to reflect this in the PreviewModel documentation
in src/preview/model.js. I would like to do this in a separate
patch.
Bug: T214970
Change-Id: I136112bfea7f732d2673bcb8c69aba9defe6ba85
This tests the newly introduced code that decides if page or reference
previews should be used in the handling of a dwel event.
Bug: T214971
Change-Id: Ib20d00b7b9ee9b1ed82763137ec62e468e8f05f9
This is mainly done to increase testabilty of this part. I am a bit
unsure if this should ( have been ) integrated in the former index.js
that's now the page.js. - See also the refactoring done before.
Bug: T214971
Change-Id: I90d0441510bc1ec0b4900a392afcbaff6a552377
This is as preparation to introduce a gateway switch that decides if the
page or reference gatway should be selected. Moving that code to it's own
realm makes that path better testable.
Bug: T214971
Change-Id: I5efa9fb8f63f1487c627eb9a3f1fe47f43c611cc
This installs a series of safety nets:
* The selector [href*="#"] skips links without a fragment.
* It's still possible that a fragment exists, but is empty.
mwTitle.getFragment() checks this.
* The gateway does not assume the element exists, but checks this first.
If there is no such element, the gateway aborts the request in a way
that no error popup is shown. This is currently only possible with the
`{ textStatus: 'abort', xhr: { readyState: 0 } }` response as seen in
this patch. We might need to introduce a new, more clean way to silently
quit a fetchPreviewForTitle() call.
* The test for the reference gateway finally covers the scraping code.
Bug: T214970
Bug: T214971
Change-Id: I9ec57e0fbb0d21beaaa7b359c1c2bef64d2c14f5
Including tests for all situations.
I believe it is impossible or extremely hard to actually abuse any of
these places. All these data are not extracted from the current page, but
delivered either by MediaWiki's api.php or a RESTful endpoint, as
configured via $wgPopupsGateway and $wgPopupsRestGatewayEndpoint. A
possible attacker would need to write it's own endpoint (which must either
run on the same server or somehow ignore the CSRF token), and set the
value of mw.config.values.wgPopupsRestGatewayEndpoint on the client to
this endpoint – which requires just *another* attack vector to be able to
do this.
It's "the right thing"(tm) to escape all this anyway.
I found two possibly relevant security reviews of this extension, T88171
and T129177, resolved in 2015 and 2016.
Bug: T88171
Bug: T129177
Bug: T214754
Bug: T214971
Change-Id: I1d118c9ccaea434a253a772d18139b9b077118ab
