Load page previews as quickly as possible.
According to performance team it should be beneficial to remove
this (See Ibc01f6a82692c7dd3d4a866354ab975af114e7b0 for more
information) and it's not a recommendation. its existence
in this codebase is leading to misunderstandings elsewhere.
Change-Id: If183d9ca07d98c03c957f359c13ca8e2ede7ad58
Certain browsers have the closest method but do not
support not selectors with multiple arguments. This
variant caters for both.
Bug: T325113
Change-Id: Ib5fc912bfe0f831fea4c9882c25b27541d83b66f
Follow up to
Iefe98c1f0422dbf034e385b1a41a859d030a2cf4 where we switched from the
jquery event delegation pattern to native methods. One thing that we
overlooked was that we also need to consider the case where the selector
matches the parent of an element, for example a span nested inside an
eligible link. I've rethought this logic, to first find the closest
eligible element to normalize the element passed to model methods
before running matchesSelector.
Bug: T325007
Change-Id: I4133751dc900a51829173e9c0d965cbb18e6a33e
Adjust the logic of elementMatchesSelector to fallback
to jQuery when the native matches fails.
Follow up to
Iefe98c1f0422dbf034e385b1a41a859d030a2cf4
Bug: T324514
Change-Id: I0e9e894e77e2eb29fee65853aa98b141bc2a11a3
Allow extensions to register new types of previews via
extension attributes.
Changes:
- The check for reference previews doesn't make sense
as $('a[ href*="#" ]' ) will match any elements with a hash
fragment, so the additional check to Title.getFragment
will not provide a different result. This was introduced in
I9ec57e0fbb0d21beaaa7b359c1c2bef64d2c14f5
- Links that point to themselves are marked with mw-selflink
in MediaWiki so this can use the not selector we already have.
- The new API is used internally and only available via extension
Attributes
- An example is provided in SkinJSON
(https://github.com/jdlrobson/mediawiki-skins-skinjson/pull/14)
Bug: T233099
Change-Id: Iefe98c1f0422dbf034e385b1a41a859d030a2cf4
Updating 'svgo' to v2.8.0 and newest Wikimedia SVG guidelines –
mainly around new whitespace features of SVGO.
Change-Id: I37da089916d2efad696989427b0a06b392d1d89c
eslint:
* Remove hardcoded glob paths that are (relatively slow) to expand
before linting could start, in favour of specifying directories
which can be iterated while linting happens.
* JSON files in i18n/ were skipped, unlike other repos.
JS code under .storybook/ was skipped.
JS code under resources/ext.popups/ was skipped.
Fix all these by doing what other repos do, which is to let
ESLint iterate the repository and tell it which directories not to
enter. This has the happy side-effect of making IDE integrations
for ESLint work correctly (as customisation in package.json has the
same problems as using Gruntfile, namely that invoking 'eslint'
directly can't be aware of this), as well as allowing things like
`eslint --fix` to be used.
```
/Popups/resources/ext.popups/index.js
4:2 error Unexpected var, use let or const instead no-var
```
nyc:
* Remove SPAWN_WRAP_SHIM_ROOT.
This was a hack for an early CI experiment that hasn't been
needed for several years.
storybook:
* Remove gitignore entries for additional npm and build output in the
storybook subdirectory, which appears to not exist anymore since
189b386a13.
Doxygen:
* Sync with cookiecutter example from
https://gerrit.wikimedia.org/g/mediawiki/tools/cookiecutter-library/+/HEAD/
- remove redundant stuff,
- enable quiet mode to hide verbose non-warning,
- include CoC.
Change-Id: If8f6b833067192aea96a87f04c7978c9af11f996
This was done while working on T277639 where we introduced
multiple "enabled" flags for individual popup types. This
change listener is one of very few places that work with
this flag. This patch is meant to make this code more robust
and easier to change.
A few unrelated but trivial changes are included that make
code shorter and hopefully easier to read.
Also fixes a bug with tooltips overlapping previews.
Bug: T287119
Change-Id: I7fb0a8d4bb9f5e78fe62cfca524cc157ea89a233
Provide containers for page previews examples
In storybook we avoid position absolute but this deviation from
how they behave in reality could lead to subtle differences that impact
the rendering of previews. It also doesn't allow to see the preview
in the context of the link which is an important part of visually
verifying the position of the pokey arrow.
This also allows us to rethink the broken RTL mode. We now scope
LTR rules to a LTR class that is present on the container, and use
the CSSJanus parsed stylesheet for RTL.
Change-Id: I189019824ddd6aa759790fd162ffcd543619a953
The requirement for landscape images is to be at least 320px
wide. The requirement for portrait images is more relaxed,
only 250px high. Images that fall between these two
requirements currently don't show a thumbnail, even if they
could.
This change affects a very specific group of images:
* Square images from 250 x 250 to 319 x 319.
* Landscape images from 251 x 250 to 319 x 250.
* Landscape images from 319 x 250 to 319 x 318.
The most extreme ratio is 319 x 250. This will be cut to
203 x 250. I.e. the absolute extreme are 58px missing left
and right, but never more.
Requested at https://www.mediawiki.org/wiki/Topic:Vwl97pm6as9fuf6k
Additional stories for testing more extreme small images:
* Small Tall - 300x1000px
* Small Short - 300x200px
Update tests accordingly.
Bug: T268999
Change-Id: I811f1c0e7e9b0c30280b36a61cc7831a5b9e58c8
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
For example: We know the HTML element we are dealing with is
always an <a> element.
I believe the small changes in this patch are all obvious and
non-controversial. Please let me know if you disagree.
Change-Id: I9fe15845affffdd0f5f0fd6ef7d6b607cb567ac7
This is not really an error. It does not really have
consequences for the user, except:
* It might waste bandwidth because the module is loaded, but
unused.
* It might waste a tiny bit of CPU time.
However, this can only happen for registered users, and only
with very specific combinations of settings (notably: a
conflicting gadget must be activated).
The reason it's marked as an error is that previously we could
give more guarantees about this. This changed recently. We will
check if the log message is still helpful and either remove it,
or restore the previous guarantees.
Bug: T271206
Change-Id: I218726c9c4879a405acef68710e79c6ac8d070fe
Open questions:
* It is possible to make the patch smaller in case this makes
it easier to backport it. Is this worth it?
* This code is not covered by any test. I think it's worth
writing tests. Most of this code will stay, even after beta.
Bug: T281352
Change-Id: I5f30054f1664643b427909f7fa189b4ea5e11879
Use mw.track instead of dedicated tracker for VirtualPageViews.
This way we can migrate it to the new Event Platform client.
The new client does not observe DNT, which was the reason this
instrument was moved to a dedicated tracker on the first place.
Bug: T279382
Change-Id: I8bb515eab337ffed686ba7522bc6153cfdd8ca8d
This does not fully solve the ticket, as these are not the
actual OOUI styles. But it's already much better than the
unstyled checkboxes before.
Bug: T281227
Change-Id: I9a5023482774c09aa73845ca6dfd1c4926f088e1
'svgo' upgraded to v2.3.0
This includes:
- Replacing .svgo.json with .svgo.config.js
- Updating the SVGs files. This amounts to changes in the order of
some attributes.
- Adding the minify-svg command as part of the npm run test command
Bug: T278656
Change-Id: Ia38332be68b8ac47a31caf30272920c0f0c12053
This is the smallest possible patch for a backport.
While it seems there is zero technical reason to limit user
option keys to lowercase with dashes, it's best-practice.
Same as message keys.
Bug: T281235
Change-Id: Ia4a45cf4459543c81b23b757ae9c2cfaf9676894
We changed the handling of the popup settings. They now contain popup types (pages and references).
To not get confused with the old setting options (simple, advanced and off),
we would like to rename the local for the simple setting to page.
It should already contain the correct string.
Bug: T277639
Change-Id: I8847b890e9e31602277a92d82a188fcdd3eea855
* Change more places to not hard-code the popup types, but use
loops and such.
* Change many `function ()` headers to use the more streamlined
ES6 sytnax.
Bug: T277639
Bug: T277640
Change-Id: Ifece87d51012e0e069286453b27f5c9ae273710e
We added reference preview as a checkbox the the
anonymous user settings. To handle both popup types
(pages and references), we changed the usage of
preview.enabled. We pass on all types as a map
inside preview.enabled. The footer link to edit the
settings will appear for anonymous users if at least
one type is disabled.
Bug: T277639
Change-Id: I860a1b35ac7749d8d0884575f6acb7186ad8e4d0
We still have 2 different mechanisms in place, maybe even 3:
* We simplify the CSS selector when we know a popup type is
disabled, and it's impossible an anonymous user can enable it
at run-time.
* We create that "initiallyEnabled" map that allows anonymous
users to toggle the individual popup types at run-time.
* This map is also used to check if the footer link should be
shown.
* There is also a wgPopupsReferencePreviews global that acts as
a "kill switch". However, this is not a pure feature flag,
but incorporates the user setting for registered users. This
is currently partly redundant (checking
`mw.user.options.get( 'popupsreferencepreviews' )` does the
same) and can be removed later when the feature flag is not
needed any more.
The footer link currently acts odd because anonymous users are
unable to enable ReferencePreviews, but get the footer link.
This patch introduces a 3-state model:
* `true` acts as before.
* `false` means a popup type is disabled, but anonymous users
can enable it (i.e. this is the opt-out behavior for anonymous
users).
* `null` means a popup type is not available at run-time, for
nobody. Anonymous users can't do anything about this.
Registered users must leave the page and change a setting.
Bug: T277640
Change-Id: Id8d1396c09cf0f706034a66f9cd3c880a8b33df8
An "advanced" option was first introduced in 2014 via patch I374805e
(originally named "monitor-or-edit", renamed via patch I7b4f6d2).
The isNavPopupsEnabled() function was added in 2016 via patch
Ic660f48.
The code that disables the extension entirely the moment the NavPopups
gadget is enabled was added in 2017 via patch Ia474b1b (T151058) and
patch Ia837816 (T160081).
As of now, the "advanced" option can only be seen in an extreme edge
case:
* Only for anonymous users.
* Only if NavPopups is enabled by default for anonymous users.
* Only if the $wgPopupsConflictingNavPopupsGadgetName setting is
misconfigured.
* … or if NavPopups is not a gadget in the first place, but e.g.
loaded via Common.js.
In this situation the settings dialog opens with all *3* options. This
is broken for several reasons:
* The "simple" option enables the extension, but doesn't disable
NavPopups. Both trigger, resulting in both popups being displayed
the same time.
* Since "simple" is the default, this bogus behavior is the default
for anonymous users.
* The "off" option doesn't stick. Every time the settings dialog opens
"advanced" is checked instead.
* "Off" can't work anyway. There is no code to disable the gadget.
* Only the "advanced" option "works", but more by accident.
It's unclear how to fix this:
* There is no code that does anything with the "advanced" option. It's
not even stored. The behavior of the option is identical to "off".
* The code appears as if "advanced" was meant to be shown instead of
"off". I.e. anonymous users can only choose one of the popups, but
not disable both. But there is no code to hide the "off" option.
* The bug when both popups are displayed was fixed in 2017 via an
entirely different mechanism. Re-introducing "advanced" does not
only mean duplication, it's unclear how the 2 mechanisms are meant
to work together.
It really, really feels like this was just forgotten.
Bug: T278949
Change-Id: Iab21f3a649a5b2f19ebb0d0dbb45ce1450c65678
… as well as in one place in production code. The motivation
for this change is to make the code easier and faster to read.
There is a little bit of duplication in the test setup now.
But I would like to argue this is a good thing. The values are
rather trivial. The difference (or absense of a difference) is
much easier to see now.
Change-Id: I9aa95b59f0c45ea7c9257970e2fcdba3a000d234
This patch does nothing but rename a pair of variables:
"prevState/state" becomes "oldState/newState". Reasoning:
1. The abbreviated "prev" is confusing, especially because we
are in a codebase that is all about "previews".
2. We are in a context that is all about a state **change**.
Change listeners get notified about the change from one state
to another. While it would be possible to stick to the already
mentioned "previous/current" terminology, I find the word
"current" confusing. What is "current" in this context? Did
the state already change? Am I notified about a change that is
**going** to happen or already happened? Is this even relevant?
I don't think it is. Therefor "old/new".
Another possibility is "previous/next".
Change-Id: Id886e1a095967fe86fb9021f59e335c62da8994e
The nextState() function was not able to understand updates that
are deeper than a single level. Example:
nextState( state, { pagePreviews: { enabled: true } } )
Before, this would replace whatever was in the "pagePreviews"
property with { enabled: true }, but not merge it. In some places
a single level of recursion was done manually because of this.
This can be removed now.
This is done in preparation for splitting the "enabled" flag into
separate ones for each popup type.
Bug: T277639
Change-Id: I35911c18018ba7cd1633a4c882b978656c3fee36
Note how getIsEnabled() is documented: "if the user hasn't
previously enabled or disabled Page Previews […] then they
are treated as if they have enabled them."
In other words: The idea that the default should be true is
encoded twice in this code. This is just not necessary. We can
remove one without loosing anything.
Motivtion: Simplifying the code and reducing the package size.
Since the code fundamentally depends on this default value
anyway, we can clear the users localStorage when they decide
to go back to the default – instead of storing a "1" which
does the same as the default.
Change-Id: I2814a1e9269979918609162a508eeee6944d9e52
The main motivation here is to dramatically reduce the number
of places that use the same property name "enabled" for values
on different objects (e.g. "state", "actions", and "updates"
are all different things) with slightly different meanings. I
tried hard to come up with names that reflect better what each
meaning is.
Bug: T277639
Change-Id: Ie766259793f716262e3d4622ca55156d11f4842c
… instead of only 2 hard-coded nesting levels, as it was before.
This is done in preparation for splitting the "enabled" flag into
separate ones for each popup type. This patch here doesn't change
any behavior or internal representation of the states.
Bug: T277639
Change-Id: Icad669d1c9675ad6de22f478e254debe5d1936d7
Add message, description, extension for title. Update createPagePreview, renderPagePreview methods to add title attribute to settings gear icon. Add test for title attribute. Increase maxSize, maxAssetSize, maxEntrypointSize. Add compiled js files.
Bug: T274887
Change-Id: Ibb29deb3418569d8283b954b4b22074423e78bda
To reduce size of code added to the <head> and increase performance.
The increased bundlesize is still less than the size spared bytes in
ResourceLoaderGetConfigVars. - But nevertheless the main gain is loading
less in the <head> anyways.
To avoid further complexity in the code, the bitmask is converted to
the according config setting early on instead of adding checks on the
bitmask all over the place.
Tests will be added in follow ups.
Bug: T276716
Change-Id: Ib4f82bed58295b25f0a41cb37e36244e45f16317
This fixes a small regression introduced in Ia61f1b7. When
reference previews are still enabled as a beta feature the
unnecessary footer is hidden, therefore, we need to account
for this when calculating the dialog's height.
Bug: T234205
Change-Id: I1c142019031ab954550e237ddb23824da1aee8db
Update copy and remove unnecessary reference preview preference
in favor of using the default preference. It seems there is no
stable method to link to the subsections on the preference page
for gadgets. So in all cases does the link just point to the
gadgets pref page.
These changes should only be visible when reference previews
are no longer marked as a beta feature.
Bug: T265709
Change-Id: I7b8ab91331092ada04b230315373548673b9272c
* 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
I added the common styling for the setting icon to the popup.less
and removed the now empty pagePreview.less.
Bug: T234205
Change-Id: I2a82831bc71a4208c4b66c18e2a4613127c43e1f
Request a larger thumbnail to ensure
the thumbnail is served scaled rather
than scaled by the browser.
Bug: T272169
Change-Id: Ibf80f24c949e14c8289b8b0a4e7369dd10ead449
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
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
This compresses much better. Gzipped it was about 400 bytes
before, and 300 bytes after.
I also noticed the icon was not even symetrical before. This is
fixed now.
Bug: T256504
Change-Id: Ic03d727662e92e36249226c5760583184fd00a43
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
AVoid the term if possible, in all internal code. The only remaining
use of the word is in the public $wgPopupsPageBlacklist config
variable.
Change-Id: Ib238731270f473ad44fcff13df617a70433f2452
* Move eslintrc.es5.json to /dist to avoid extra Grunt config
* Upgrade clean-webpack-plugin and exclude dist/.eslintrc.js
from cleaning
* Set root:true and just enable wikimedia/language/not-es5 instead
of disabling dozens of rules
* Remove getOwnPropertySymbols rule as webpack uses this.
Change-Id: I802138a8a591dd4c3cb0cc637112e383570286df
This means that users of the mobile site on a desktop browser will
now benefit from Popups.
Targets is not necessary for `ext.popups.images` as these modules are
enabled on mobile by default.
Bug: T236097
Change-Id: I401fbb522ec97fdc81259702c8283c95386531af
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
