Commit graph

324 commits

Author SHA1 Message Date
WMDE-Fisch 190962cc5b Remove dead code around cog wheel dialogue
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
2021-04-15 15:29:16 +02:00
Andrew Kostka 398565b1d6 Track anonymous enables/disables of reference previews
Bug: T277641
Change-Id: I89ee3ff280e79f560bf613a110a1c9e0b3ba6648
2021-04-15 14:21:32 +02:00
Thiemo Kreuz 622fb6c5e8 Allow to disable/enable popup types individually
Bug: T277639
Change-Id: I0742b8ab1c5ed0b374d4f9447bebc46a35207339
2021-04-15 11:30:36 +02:00
Thiemo Kreuz 4b1f020b9d Avoid hard to read jQuery.extend() in tests
… 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
2021-04-15 10:47:44 +02:00
Thiemo Kreuz b28f48d6d6 Rename variables in change listeners for clarity
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
2021-04-15 10:19:40 +02:00
Thiemo Kreuz a35e35e3b3 Expand userSettings module for reference previews
Bug: T277639
Change-Id: I13acfb8bcc6e95fc28969072ec5420fd075d5096
2021-04-13 12:19:49 +02:00
Thiemo Kreuz 51dd4b2807 Make nextState support non-flat updates
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
2021-04-12 13:46:18 +02:00
Thiemo Kreuz ddf574afa3 Remove not needed userSettings.hasIsEnabled()
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
2021-04-08 11:57:52 +02:00
Thiemo Kreuz c5accc0300 Rename many functions and files for clarity
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
2021-04-08 11:04:02 +02:00
Thiemo Kreuz 1f052e0dcb Make syncUserSettings support dot-separated paths
… 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
2021-04-06 17:46:21 +02:00
Clare Ming 4908f82c01 Add title attribute to settings gear icon
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
2021-04-02 13:30:07 -06:00
Noam Rosenthal e4ee4ed5cf Set thumbnail width to 215px
When it was min-width: 215px, it could sometime
grow and break the popup's layout.

Bug: T276668
Change-Id: If88887920c48c0a7f6530d1a7600f25fd2a9579b
2021-03-22 14:40:27 -07:00
WMDE-Fisch 58f820009e Use flags to represent settings
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
2021-03-11 13:38:06 +01:00
Andrew Kostka 9fcdaf5c00 Fix maximum height of reference popups when in beta
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
2021-03-05 12:52:13 +00:00
Svantje Lilienthal fb0863f101 remove spacing, when the footer is empty
Bug: T276200
Change-Id: Id5d30a53251bdf3674388cbdb29d93a0d52b3266
2021-03-04 16:22:55 +00:00
Andrew Kostka 82d54945e4 Combine page and reference preview preferences
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
2021-02-25 10:21:16 +00:00
Thiemo Kreuz 513899d808 Much more relaxed reference type detection
* 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
2021-02-22 11:55:03 +01:00
Svantje Lilienthal 1962372a44 Add cogwheel to disable ReferencePreviews
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
2021-02-16 15:39:43 +01:00
Thiemo Kreuz ef4b032513 Add new reference type "note" without an icon
Bug: T274343
Change-Id: Ib9f3fe98baf1d194e686b007a9c535f2b49ac19d
2021-02-11 10:15:36 +01:00
Noam Rosenthal 83c528c7e1 Fix bluriness on low-DPR screens
Request a larger thumbnail to ensure
the thumbnail is served scaled rather
than scaled by the browser.

Bug: T272169
Change-Id: Ibf80f24c949e14c8289b8b0a4e7369dd10ead449
2021-01-26 09:49:50 -08:00
Thiemo Kreuz 8fbcf6f962 Add missing TextExtracts parameter sectionformat=plain
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
2021-01-08 09:17:35 +01:00
Noam Rosenthal 9d1d5c5fc2 Make CSS.supports more resilient
Following this comment:
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Popups/+/647002/5/src/ui/renderer.js#163

Making sure CSS.supports is a function
Before calling it for graceful degradation

Bug: T269336
Change-Id: Iee4050ae14e28667eb8014c223eb925018df7bd3
2021-01-06 15:43:54 -08:00
jdlrobson 1679eea477 Fix settings dialog display
Follow up to 23b6332a5b

Bug: T270650
Change-Id: Ic128e84eed69e4fdc7252bb6b315bd3f953eaf73
2021-01-05 12:20:06 -08:00
Noam Rosenthal 7b0937c5a8 Use CSS clip-path instead of SVG when supported.
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
2021-01-05 19:26:24 +00:00
Bartosz Dziewoński c8b8ba7f5f Revert "Remove title attributes at init"
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
2020-12-11 16:53:17 +00:00
Svantje Lilienthal cbd4e673d4 Add qunit test for conflict with popups
Bug: T267951
Change-Id: I47cc8e8565252dc78e39c00c94a07300b4e35593
2020-12-10 15:47:38 +01:00
Noam Rosenthal 497eb631d1 Parse template HTML only once, as HTML parsing is expensive
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
2020-12-07 23:01:26 +00:00
Vedmaka ff2ba9ebf5 Adds "$wgPopupsTextExtractsIntroOnly" option to control "intro" parameters for TextExtracts API request
Change-Id: I84f91ab0752bbd4a5b1ee159b7cbe1388f942f24
2020-12-07 15:00:29 -08:00
Noam Rosenthal 4492b54a44 Performance optimization for popup rendering
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
2020-11-30 17:33:07 +02:00
Noam Rosenthal 6bc2077ed5 Remove title attributes at init
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
2020-11-30 15:39:36 +02:00
Svantje Lilienthal 0b6c859a2b hide reference previews when reference tooltip gadget is active
Change-Id: I5f43270bfeba944c05dc09adf771ed07057237c7
2020-11-16 18:36:04 +01:00
jdlrobson 140b1e5828 Settings cog should come from icon pack
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
2020-11-12 05:57:57 -08:00
Adam Wight 789cedc168 Remove reference preview "Jump to reference" link
Now that the footnote label links to the references section, we don't
need this additional link.

Bug: T265482
Change-Id: Ib9b2939eb49e7b826c7699a5b7fa0e8255fa9da1
2020-10-30 11:33:24 +00:00
Thiemo Kreuz c1abe80b08 Minor code cleanups, e.g. utilizing arrow functions
Change-Id: I56bcfa040553a96f018f22483f3f988c5639fc97
2020-10-30 11:31:12 +01:00
Thiemo Kreuz 7d2de31fd9 Don't show preview popups with no visible content
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
2020-10-30 11:19:52 +01:00
Adam Wight 877c2f3e12 Clicking on a reference should behave normally
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
2020-10-30 10:57:41 +01:00
Thiemo Kreuz f4d696e6bf Add (i) info icon to collapsible replacement message
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
2020-10-30 10:53:30 +01:00
Thiemo Kreuz 1cf721e2a2 Handle collapsible & sortable elements in reference popups
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
2020-10-28 17:23:12 +01:00
Thiemo Kreuz 26ca2bcf5b Scroll reference heading as well
Bug: T249548
Change-Id: I808ee9aadc8766490e98267bb94d25887f404362
2020-10-28 11:26:52 +01:00
Thiemo Kreuz d6ab2072e3 Open only external links in new tabs
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
2020-10-20 10:52:05 +02:00
Umherirrender 23b6332a5b Avoid shadowed variables in javascript
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
2020-09-21 20:31:44 +00:00
jdlrobson 5dfb0ea04e Fix TypeError: u.abort is not a function
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
2020-09-03 10:16:17 -07:00
Ed Sanders 56d87d6795 Disable popups inside VE surfaces
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
2020-08-08 12:53:03 +01:00
Thiemo Kreuz 5da88be420 Disable Reference Previews in the (mobile) Minerva skin
Bug: T243822
Change-Id: I6de02243301184bce8ff53a56b13752880ecfbe6
2020-07-24 13:01:47 +02:00
Adam Wight 07aa0f3ae9 Don't record Popups actions on non-content pages
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
2019-11-14 16:52:05 +01:00
Adam Wight 0c1e628edb Ensure that pageviews are recorded at most once
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
2019-11-08 11:23:02 +01:00
Adam Wight b7331a6e66 Record pageviews where Reference Previews is enabled
We want to know the side of this population in pageviews, in order to
produce proportional metrics later.

Bug: T214493
Change-Id: I940872870754e85d19688f3855d6857e65b982b1
2019-10-24 23:34:56 +02:00
Adam Wight fed99e7cb5 Inline eventlogging call
This isn't messy enough to justify encapsulation in a function.

Change-Id: Idfa6581d30709f1a58ff8c83ead08e1bfda81c1c
2019-10-24 22:52:48 +02:00
Adam Wight 171e1b7dbf Sample ReferencePreviewsPopups 1:1
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
2019-10-22 12:14:17 +02:00
Thiemo Kreuz 76e02fae98 Remove obsolete mediaWiki and jQuery aliases
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
2019-10-22 09:30:46 +02:00