This reduces a lot of churn in creating the SVG
element and related helpers.
When IE11 is dropped, the SVG code-path can also be dropped.
Bug: T269336
Change-Id: I7f91192dedc2a78f1c7c84179cff1687593177c0
This reverts commit 6bc2077ed5.
The change causes issues with various popular gadgets on Wikimedia
wikis. The 'title' attributes have been the easiest way to determine
the target pages of links, and many gadgets have come to rely on that.
Bug: T269297
Bug: T269873
Change-Id: I49d315a13c327a1c5af51d3de887c0c17642e9fe
When creating a popup, clone the previously created DOM element
and populate the attributes and content.
Ideally this would be done with a template element, but since IE11
is still supported this is not possible.
Change-Id: I347615cf1f613d97d767d60627b13b6b3ff9c762
Bug: T269338
Reduce layout/style thrashing by measuring all required geometries
at event handler, not waiting for delays/redux/style changes.
Use CSS bottom instead of top, to avoid having to measure the popup
before positioning it, if it's placed above the link ("flippedY").
Disable some test cases that relied on implementation detail of using
"top" CSS.
Change-Id: Id0cbf506009b824d0fb6af4d6fe220e2f69aaaad
This is a performance optimization - removing
all the titles when initializing the popup extension
reduces DOM manipulation during hover, removing/reinstating the title
attribute.
When the popup extension is loaded, the default "title" behvior is unnecessary.
Change-Id: I1a85394b6b67eabee50a8d554bfd9b62de2a24d3
To avoid continuously updating this cog, use the icon pack directly.
Use mw-ui-icon-small to control the size rather than custom CSS - this
reduces the amount of CSS overrides that are needed.
Also use `opacity` instead of icon SVG fill for coloring the icon. This
enables simple transition in interaction states.
Storybook: The settings cog will now be tied to the production icon.
Note for now this will not appear at all, as this code must first
have ridden the train. For local testing feel free to point to
localhost to verify this change.
Bug: T256504
Change-Id: I2a28666dbd644bb599146fabb84d148ff0167ed3
Now that the footnote label links to the references section, we don't
need this additional link.
Bug: T265482
Change-Id: Ib9b2939eb49e7b826c7699a5b7fa0e8255fa9da1
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
We no longer intercept reference clicks, now clicking on a reference
label will scroll to the reference definition in the same way as when
reference previews is disabled.
Metrics about clicking on the label are collected by the Cite
extension, are are unaffected by this change.
Bug: T265482
Change-Id: I2929a86b6a09f3b72e5e2f4151cb13f52446897d
This avoids pulling in the entirety of OOjs, with the disadvantage
that we have to copy a little bit of CSS. I copied parts of this
patch from I2a28666.
There might be a better way to do this, with less code. E.g. is
there a better way to construct these HTML elements?
Bug: T220208
Change-Id: I024155f3ff0f57de1d68bbaf37bfb9e81e692bd0
Elements that are marked as collapsible (often tables, but can
actually be anything) are most certainly marked as such because
they are big and don't fit in a popup.
Another plugin makes tables sortable.
In both cases non-functional UI elements appear in the popup.
We decided:
* Hide collapsible elements (no matter if currently collapsed
or not), and show a placeholder text instead.
* Remove sortable arrows.
This is a baseline patch that solves everything, except the
(i) icon is missing. This will be added in the next patch.
Bug: T220208
Change-Id: I58f3036bf4988d0ebe5716b0a54506446fca10c3
There are 3 types of links:
* Links with no class are links to other pages in the same wiki.
* Links with "extiw" are interlanguage as well as interwiki
links to other known wikis (e.g. from Wikipedia to Wikidata).
* Links with "external" point to somewhere else on the web.
Bug: T215419
Bug: T239230
Change-Id: Ia25db94d02670a919fc003f3e3562725de2e8eae
The rule no-shadow gets enabled in the version eslint-config-wikimedia
0.17.0.
In prepration for the automatic update with libup this needs to be done
in a separate patch set due to T262450
Change-Id: I66d405aef6d2b777e9a7f63734f2b5a5649d2080
The abort method is not always present and the xhr may not be abortable.
This mirrors the 'abort' in promise check later in the file.
Bug: T259652
Change-Id: I7dad89b083d6a0a83ffc59e29af8942cb0eaf640
This was already accidentally the case for the main VE article
editor as it doesn't live inside #mw-content-text, however other
VE surfaces (e.g. DiscussionTools') don't live inside #mw-content-text.
Bug: T259889
Change-Id: I48bdee6f62af33cad7512128b5465d474a6dfebb
We found that Popups was overcounted approximately 2.5x relative to Cite.
This patch attempts to nearly match the circumstances under which Cite
(and its tracking) is loaded.
Bug: T214493
Change-Id: Ib31df3c33879f4ea63d9808ffd260861069a8977
The Popups pageviews were suspiciously high, about 2.5x higher than
Cite counts which should have reported the same number. This patch
attempts to deduplicate pageview events.
Bug: T214493
Change-Id: I51bf6d1909d65ecd9d3ef28eda285852897343ec
We want to know the side of this population in pageviews, in order to
produce proportional metrics later.
Bug: T214493
Change-Id: I940872870754e85d19688f3855d6857e65b982b1
We're getting a few dozen records per day at 1:10, so let's stop sampling.
We need enough data to get significance on events happening at 0.1% or less.
Bug: T214493
Change-Id: I8a913fe1ee1e5b72d84914e183ac2386ddb20d84
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
We've logged zero events after 12hr of deployment, so it should be
safe to increase sampling by 100x.
Bug: T214493
Change-Id: Icd67aed448269f603dd4465f7e46eac9a64bd1ab
Logs events to the ReferencePreviewsPopups EventLogging schema, in
order to understand whether Reference Previews is helpful for
end-users.
This will be removed along with the older tracking, as soon as our analysis
phase is finished.
Incidentally disables a lint rule for the generated JS, it's about
readability so irrelevant to the minified code.
Bug: T214493
Change-Id: I2638611ba67b785338f7e98a1c4b08a5e829812d
The eventlogging `isEnabled` function determines when to sample,
this patch removes the `isAnon` conditional so that we can measure
reference reading habits for all users.
Also guards against a non-function navigator.sendBeacon, which was
previously intended but incorrectly tested.
Bug: T214493
Change-Id: I42cb3082fb85c7900426a2055dfa3c2f6ecfd968
I was able to track the issue down to the changes made in this patch:
https://gerrit.wikimedia.org/r/331563
This patch is mostly a simplification of ce8a2d4
(I9a73b3086fc8fb0edd897a347b5497d5362e20ef):
- Don't make wait#wait() abortable. This adds complexity and isn't
needed since the token is rechecked in the .then() of
actions#linkDwell(). The request is permitted to continue and fetch if
that token still matches and is never issued otherwise.
Once a request has been issued, that request is still abortable.
However, note that calling XHR.abort() is just a request to abort and
may not be granted. Whether or not XHR.catch() is invoked is what
dispatches the FETCH_ABORTED action (or doesn't if the request to
abort was denied).
- Remove the abort tests for wait#wait() since the functionality is no
longer provided.
- Pass the preview token in the FETCH_ABORTED action and reduce
FETCH_ABORTED the same way as ABANDON_COMPLETE in reducers/preview.
The follow-up patch, I3597b8025f7a12db0cf5d83cce5a77abace9bae3, adds
integration tests for the specific bug fix. Note that these Selenium
tests are incompatible with the content proxy, so it is probably best to
simply unset $wgPopupsRestGatewayEndpoint and $wgPopupsGateway and allow
the defaults to be used. Also note that the tests are incompatible with
recent versions of Node.js (so use NVM) and emit many deprecation
warnings (so set deprecationWarnings to false in
tests/selenium/wdio.conf.js) An example run of the tests looks something
like:
chromedriver --url-base=wd/hub --port=4444 &
# If any changes are made locally, also run `npm -s start &`.
MW_SERVER=http://localhost:8181 \
MEDIAWIKI_USER=foo \
MEDIAWIKI_PASSWORD=bar \
DISPLAY= \
npm -s run selenium-test
Live testing may be performed as well. Remember that RESTBase requests
are incompatible with MobileFrontend's content proxy hack so ensure to
comment it out if $wgPopupsGateway is configured for RESTBase (see
T218159).
1. Open the DevTools network tab.
2. Disable the browser cache. Chromium, at least, won't abort requests
coming form the cache.
3. Hover back and forth quickly over a preview. In Chromium, canceled
requests are labeled and appear red. This is a good scenario to test.
With the patch, a preview should always be shown when ultimately
resting on a link. Without the patch, it is possible to rest upon the
link with no preview showing. This may require several attempts.
Bug: T219434
Change-Id: I9da84b0296dd14e9ce69cb35f1ca475272fb249a
As mentioned in T205744, EventLogging schema ResourceLoader modules have
been deprecated. This removes those modules from loader calls.
Bug: T221281
Change-Id: I1b7355c69e09756f50ccd1c1955b45cae4a64b9e
Removing wrong ARIA `role=tooltip`. Those are meant only for static
text content on short `title` like dynamic tooltips.
Bug: T223827
Change-Id: Ib4f490c7ad421e516fb0cf47eff4335bcaf26c43
We did a strict === comparison before. This works fine when the page
zoom is either 100% or 200%. Other zoom factors produce fractional
numbers. Comparing them with === is doomed to fail because these numbers
are IEEE representations, not necessarily being identical in situations
where you would expect them to be identical.
This applies two fixes: Change the comparison to a >=, and always ignore
1 extra pixel. If we are so close to the bottom, the gradient is not
helpful any more.
Change-Id: Idf4c604142de8d2a803e1d94f3093cec8a8abb72
This fixes page previews appearing when there is self link to a page
that has a file extension in it's name.
Bug: T222869
Change-Id: I5caf610fa76986026948a5b7b55537723752b755
This is relevant in case the HTML looks like this:
<… class="reference"><a><span>[</span>2<span>]</span></a></…>
The additional <span> cause many additional mouseover and mouseout
events. The code already tries to filter these duplicate events that
are all triggered on the same link, but gets confused, especially when
the multiple chains of events overlap each other in unexpected ways.
It's a timing issue. This change does not fix the fundamental issue,
but does make it much less painfull.
This patch also removes the > from the selector. This is in case the
HTML looks like this:
<… class="reference"><span><a>[2]</a></span></…>
The additional inner <span> would prevent any reference preview from
being shown.
Bug: T214693
Change-Id: If2554ba78072245c27a1f85c46f33e3c58582c1d
Introducing the REFERENCE_CLICK action that will fetch and show the
preview for the clicked reference right away without any delay.
The main goal of the new chain of events introduced with the reference
click is showing the reference preview right away. The actions triggered
by the dwelling include delays in multiple parts of the process.
If there's a dwell action-chain in progress when the click action is
executed, the related promise ( that might still include steps with
delays ) and the reference preview is retrieved and shown right away
re-using the token.
In the case where there's no dwell action running ( e.g. when the click
was triggered via touch ) we create a new token and start from scratch.
In either case we want to avoid, that multiple clicks trigger multiple
actions and abort early when there's already a click action in progress.
Bug: T218765
Change-Id: I073a93be2d17a21178aebe12267765f60a2811b9
I actually tried to bring the animation for the "bottom" property back.
When finished, I realized this animation is done on *top* of the
horizontal scrollbar. This looks fancy, but is not what we want. We
need to keep animating the opacity.
This patch here contains a bit of refactoring that was left after I
went back animating the opacity.
Bug: T220200
Change-Id: Icf613f72f3baa3de86f8aa319667c8e8f16593fd
The changes in the patch check if a horizontal scrollbar is present
and move the absolute positioned fade overlay above that scrollbar.
Since we cannot change the CSS of the :after element via JS a new div
element was introduced.
Bug: T220200
Change-Id: Ia69c9be0facaf3ecebdb9f76d36f7cb3412c0816
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
config.get( 'wgTitle' ) returns the unnormalized title of the current
page while title.getName() gets the normalized title (e.g. with underscores ).
On pages with spaces in the check failed before this patch.
Bug: T220097
Change-Id: I58a532627bb27be030cbc553f1181a89109edd80
Named references may include non-ascii characters, so we decode the fragment before matching against reference IDs.
Bug: T220196
Change-Id: I63bba59fa8f0f6aa95aeadbb1f85745d480988bd
It is set based on the same conditional that loads the code,
thus checking it inside the loaded code is a no-op and adds
extra HTML to the <head> that blocks text/layout rendering and
delays fetching of Popups JS.
Bug: T219342
Change-Id: I9c1f4b3861ce2cecb654eb0a78469a616730a40b
