Turns out this was only in place because the test was
(intentionally) incomplete. But it's dead code in
production. Let's get rid of it.
Change-Id: Ieeb145b6972dceb7eeda3a167d907b680d5c3ce4
* When there are multiple <cite> elements, the first that contains a
known class is used. We assume the earlier one must be the relevant
one.
* When there are multiple known classes in e.g. <cite class="web news">,
the last one is used. This follows how CSS works.
* There is extra validation if the corresponding message exists, just
to be sure. This is cheap.
Bug: T274979
Change-Id: Iee3481ea16af96b40faf978e254718e5a48917f3
It turns out the TextExtracts extension is build in a way so
that the parameter plaintext=1 alone is not enough. It does
not really mean "please return plain text" but "please don't
return HTML". It can still return one of three formats:
* Wikitext.
* An intermediate parser format where headlines are not
`== this ==` but `��2��this`.
* Actual flat plain text.
The default is wikitext. That's why we see `== wikitext
syntax ==` in certain edge-case situations. Forcing it to
"plain" fixes this.
Bug: T271439
Change-Id: I3035fb3df99680af8bbd10c4513aed730013c344
I played around with a lot of options, and settled with:
* When there is nothing but text, but the text is all
whitespace, don't show it.
* Make sure an <img> with no text is still shown. This is done
by checking for child elements.
A possible future enhancement could be to check the visible
width of the element as well. Unfortunately this fails in
tests. Everything is 0 in tests.
Bug: T240543
Depends-On: I2929a86b6a09f3b72e5e2f4151cb13f52446897d
Change-Id: I94ed575abcd69241c82480ade07017e61cc26c9c
Notice how this actually reduces the size of the final, compiled index.js.
It's not much, but still.
One issue I noticed is that the coverage reports for the JS code stopped
working. I have no idea why.
Bug: T208951
Change-Id: I2fe92579574b3b1ba4d2dd064899eee944045a96
The signature mw.RegExp.escape() is deprecated since MediaWiki 1.34.
By the way, this is the 4th or even 5th time in my short career this
tiny, single line (!) helper function is moved around and I need to
update all my code. This felt already ridiculous when it happened the
2nd time.
Change-Id: I4d05a49120aff64ebc316d0af7736c62385d9307
Pretty much all usages of this function do *not* use the second
parameter to pass a page name prefixed with a namespace, but pass the
page name only.
This patch here removes the redundancy. The namespace prefix is now a
generated one, saying "Namespace <number>:…". It turns out no test
relies on this so far.
Bug: T220097
Change-Id: Ibd45d49c91e86a2647afe676a5e3bb07dfeab6ed
The method itself has not much to do with gateways as such, it's
more about the general preview type selection. Since the preview
types "live" in the model, I thought it might good to move it there.
Doing that the "original" getPreviewType method in the model was renamed
to avoid conflicts. If I get this right, that method is quite specific to
page previews, since it processes the output from the TextExtracts API-
request. - Therefore I also removed the TYPE_REFERENCE there, because this
code path will never be reached with that type afaik.
Inspired by the comments in Id1fa7dad59d8fe80bc60c1e2d7c3fb4087e52d1f and
as preparation for that patch.
Bug: T215420
Change-Id: Ic9e24a73e945c7d56435c656ecfdb42b65601d22
Rename selectInitialGateway() to selectGatewayType() in
gateway/index.test.js. This test's import name was out of sync with the
declaration which made it more difficult to grep and reduced
consistency.
Change-Id: I60ae359043fdf76015d75b6438992ef6061a4d72
This happened one time before and was fixed via T212419. It now came
back after I2670331, a patch for T214861 that made the title parser
accept more links than before.
This patch implements a decision tree that goes as follows:
1. If it's a link to another page, it can't be anything but a page
preview, as having a peek into another page is effectively the
definition of a page preview.
2. Otherwise it's probably a reference preview, but only if:
* It's a link with a jump mark.
* The link is actually marked as being a "reference".
* Reference previews are enabled.
3. If neither of these definitions fit, do nothing. Note this is not
"hiding errors" we would be able to fix in this code. There are many
ways to manually arrange wikitext in a way that it looks like one or
the other, without fulfilling all requirements. Unfortunately the
user who would see error message or dysfunctional popups would not
be the same as the user who wrote such wikitext.
Bug: T216683
Change-Id: I8d021f19ddc73a261e6a0c62959ddd0cb1d3182d
While reviewing the code of the reference endpoint I found it encodes
two CSS class names, "mw-reference-text" and "reference-text". So far
our scraping gateway only implemented one.
https://phabricator.wikimedia.org/diffusion/GMOA/repository/master/
This patch also extracts a piece of code to a named function. This makes
it much easier to read, I feel.
Bug: T214908
Change-Id: I9d1bb1f4c21eb9d57a6b763ca1f756e6cf7049e0
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
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
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
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 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
This will affect all links, including [[Other page#Fragment]] for
example. But it will not have much of an effect there. The mw.Title
class is able to understand strings like "Other page#Fragment". All
old code calls title.getPrefixedDb() on the result. This will *not*
include the fragment. Only the new code will use title.getFragment().
I made sure this does not affect regular page previews, even when the
link is something like [[Other page#Fragment]].
Bug: T213415
Change-Id: I15611a44aa0477cc5e48ee4b12aae3cd981d977c
I guess both is fine: either having the default in the gateway (as it
was before), or in the renderer (as this patch proposes). I, personally,
feel better with having it closer to where it is needed. This way it's
not possible to accidentially deliver a model object with an empty title.
The renderer will catch this.
At the moment we don't know exactly how we will fetch other titles (e.g.
"Book").
This change is split from I15611a4 where it was a little misplaced.
It also includes a test for the default fallback title.
Bug: T213907
Change-Id: I8ec3ddc21a417da7f95feff7b080cbd60d5472e7
Excluding tests for the renderer which keeps failing. This will be
readded in a later patch.
Bug: T213415
Bug: T213908
Change-Id: If79fa3d0a7a20f121b1ceda6e0e33ad691b1ad30
It's not exclusively about page summaries any more.
We had a few suggestions in mind:
* get, fetch, request, or issueRequest. But I feel these are all to
generic and don't describe well what the method does. As a reminder:
It expects a Title object and returns a promise, which returns a
PreviewModel object, which contains an HTML "extract".
* fetchPreview? I feel this can still mean to many things.
* fetchPreviewModel? But we don't really need to repeat that it will
return a model object.
So I went for fetchPreviewForTitle. What do you think?
Bug: T213415
Change-Id: Icb32c63cec82f72453dc1507c9f8b8d461fd4f4c
This is split from the current draft patch Ie0ccb03. This is part of a
series of very small patches that prepare the code for new types of popups.
We decided to not add code for other types of popups to the existing
createGateway() function, but introduce new files and functions instead.
Renaming, for example, the existing `gateway` variable name will make it
much more obvious which of the future gateways does what.
Bug: T213415
Change-Id: Ifcbc3ba53d0ab9ef67adf1f314defc76b4f89e89
This is split from the current draft patch Ie0ccb03. This is part of a
series of very small patches that prepare the code for new types of popups.
Bug: T213415
Change-Id: I00d46a716c0e6ada82ffc0034a7dd5582363c657
* Force arrow-parens
* Disable no-prototype-builtins for time being
* Drop unnecessary maxlen rule
Change-Id: Iceb0fe47354a5753202d2c6ad9e1a9c76791f744
Previous implementation did not pass the `result` variable
to the catch() statement. Because of that every execution that
ended with exception inside fetch() statement was threated as
not a network exception and tried to present the null preview.
Changes:
- properly handle data returned by rejected fetch promise
- chaged the big if (result && result....) into something easier
to read
- pass Error object instead of 'http' string
- Restbase can return exception, it doesn't have to handle the 404
errors by itself, it's already taken care in the catch() logic
- fixed unit tests to reflect new logic in restbase gateway
Bug: T199482
Change-Id: Ibb30fc58248623d9ad4c5388a5b2ff9b387e01de
Instead of mixing window.mediaWiki / mediaWiki and window.jQuery /
jQuery references, always refer to globals which exist whether code is
executed in browser or headless Node.js environments.
find src tests -iname \*.js|
xargs -rd\\n sed -ri 's%window.(mediaWiki|jQuery)%\1%gi'
Change-Id: I21d0a602dcbd2bc6774934bee6c487e443270fe0
Changes:
- added acceptLanguage as a config option passed to
both mwApi and restbaseApi, by default code will use
the language defined in `wgContentLanguage` config
variable. The `wgContentLanguage` is always defined
(see ResourceLoaderStartUpModule::getConfigSettings())
so there is no need for checking the variable existence.
The new logic was tested both on MediaWiki API and Restbase API
Bug: T198619
Change-Id: I1cb31f1999fd674a8b870b2b5effb92ed3dfaa1f
Whenever an HTTP request sequence is started, i.e. wait for the fetch
start time, issue a network request, and return the result, abort the
process if the results are known to no longer be needed. This occurs
when a user has dwelt upon one link and then abandoned it either during
the fetch start wait time or during the fetch network request itself.
This change is accomplished by preserving the pending promises in two
actions, LINK_DWELL and FETCH_START, and whenever the ABANDON_START
action is issued, it now aborts any previously pending XHR-like promise,
called a "AbortPromise" which is just a thenable with an abort() method.
There is a similar concept in Core:
ecc812f06e/resources/src/mediawiki.api/index.js.
Aborting pending requests has big implications for client and server
logging as requests are quickly canceled, especially on slower
connections. These differences can be observed on the network tab of
DevTools and the log in Redux DevTools.
Consider, for instance, the scenario of dwelling upon and quickly
abandoning a single link prior to this patch:
BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_END STATSV_LOGGED ABANDON_END EVENT_LOGGED FETCH_COMPLETE
And after this patch when the fetch timer is canceled (prior to an
actual network request):
BOOT EVENT_LOGGED LINK_DWELL ABANDON_START ABANDON_END EVENT_LOGGED
In the above sequence, FETCH_* and STATSV_LOGGED actions never occur.
And after this patch when the network request itself is canceled:
BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_FAILED STATSV_LOGGED FETCH_COMPLETE ABANDON_END EVENT_LOGGED
FETCH_FAILED occurs intentionally, STATSV_LOGGED and FETCH_COMPLETE
still happen even though the fetch didn't complete successfully, and
FETCH_END doesn't.
Additionally, since less data is transmitted, it's possible that the
timing and success rate of logging will improve on low bandwidth
connections.
Also, this patch tries to revise the JSDocs where possible to support
type checking and fix a call to the missing assert.fail() function in
changeListener.test.js.
Bug: T197700
Change-Id: I9a73b3086fc8fb0edd897a347b5497d5362e20ef
Prevents video files and other non-image files from being rendered as
popup thumbnails. Restricts thumbnail format to either jpg, png, or gif.
Bug: T193792
Change-Id: I7a9be5d1c8396c02ebf0893c960f65644acc9d99
Via jscodeshift:
jscodeshift \
-t jscodeshift-recipes/src/qunit-assert-equal-to-strictEqual.js \
Popups/tests
Also, some very minor manual clean up.
https://github.com/niedzielski/jscodeshift-recipes/blob/5944e50/src/qunit-assert-equal-to-strictEqual.js
Additional change:
* Drop redundant clipPath parameter from createThumbnailElement - this
parameter does not exist in the function signature.
Change-Id: I209ecf2d54b6f5c17767aa2041d8f11cb368a9b5
Creating a different page preview for disambiguation pages.
This patch:
- modifies the Preview model to accept a new 'type' property
- modifies the Restbase Gateway to pass the 'type' prop to the Preview model
- creates a new template to accept both generic/disambig previews
- modifies the renderer to render the new template
- generates icons for new template through resource loader
- adds new i18n strings
- modifies event-logging "preview seen" event to send new "disambiguation" previewType
- updates event logging schema version
- adds tests for Preview model and renderer for new preview type
- does way too much? yes, yes it does.
Bug: T168392
Change-Id: Idc936cc3eabbdd99a3d98f43c66b4cdbb7d24917
Allow developers to use different endpoints for summaries
= developer happiness
This is useful for the following use cases:
* A developer wants to test against a production endpoint via
CORS
* A developer has setup an API where REST is hosted elsewhere
e.g. http://localhost:6927/en.wikipedia.org/v1/
* A user wants to create their own REST summary compatible
endpoint
* A wiki e.g. wikidata wants to use a different endpoint which is compatible
with the summary endpoint.
We are unlikely to use it ourselves on Wikimedia wikis (the
default should suffice) but this will be a powerful tool for
When not configured this will continue to work as per normal
Change-Id: I8a7e12fbc43cddbac678e0d7b81d1e877b747b22
Stripping parentheticals were designed specifically for working around
issues with content inside wikimedia wikis and error prone.
This problem for wikimedia wikis is solved by the mobile content
service.
Given we have no intentions to use the MediaWiki API for summaries.
They are not necessarily useful to third parties and it makes little
sense to maintain them (a third party can configure their own API or
use their own REST endpoint if they really do need them).
Bug: T189042
Change-Id: I2729dc9f172af0afee1c6f0cd563c556b4ae0aeb
Don't assume that thumbnail URLs contain a dimension delimiter of "px-".
Previously, thumbnail URLs always contained the width. e.g.:
https://upload.wikimedia.org/wikipedia/commons/a/aa/100px-Red_Giant_Earth_warm.jpg
However, thumbnail URLs that actually point to the original are not
sizable:
https://upload.wikimedia.org/wikipedia/commons/a/aa/Red_Giant_Earth_warm.jpg
These are provided, for example, when the thumbnail size requested is
larger than the original. There was code designed to handle this
scenario but it only applies when RESTBase and page preview thumbnail
sizes happen to be in sync. In other words, if RESTBase requests a large
thumbnail on behalf of page previews, and page previews only requested a
small thumbnail, the original may be unexpectedly provided. A
conditional is introduced in this patch to verify that "px-" is actually
detected. If it is not present, the original is used.
Bug: T187955
Change-Id: If4e29dd870aecd6d461cc8203f6576d1bb8844f2
