The behavior for PagePreviews – implemented years ago – was
different. For old users it was disabled by default. This
behavior is wrong for ReferencePreviews.
Bug: T271206
Change-Id: I28c43ab3348ffdc27b068fc1ac1d6a6a06ee90a6
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 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
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
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
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
In this patch we introduce a new config variable and update its default
to be the same as the production value of $wgPopupsPageBlacklist.
When this code is deployed everywhere, we will remove wgPopupsPageBlacklist
from mediawiki config and defer to the Popups extension as the source of
truth from now on.
Bug: T254676
Change-Id: Ifebdcf8f5eec854a2b947dc390eaf47704a5c5eb
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
Remove pluralization from the service wiring filename for consistency
with similar files in other Web repos. There are multiple services in
this file but the point of distinction doesn't seem worthwhile.
Change-Id: I6ff4f9caf66a6156f6aa6a8b808f51c356df3414
This is a direct follow up for I4d31805. Note the previous patch was
only testing the code path when Beta is disabled. But the bug was about
what happens when Beta is enabled. This is now properly covered by
these tests.
Bug: T240947
Change-Id: I54c5e9751e14a94808e85d935e1972eea0395002
It appears like we accidentially copy-pasted the behavior for
PagePreviews and made ReferencePreviews behave the same, not taking
into account that the later feature is in Beta mode.
Bug: T240947
Change-Id: I4d31805ee9b2045c49c7ab4179c5a4adbcba0394
This user option is currently used in the BetaFeatures tab, but we
are going to change this with T233813. We are going to make this a
regular preference on the rendering tab, and would like to keep the
name "popupsreferencepreviews" along with all decisions the users
previously made.
Bug: T233813
Change-Id: I703d8a1a4eee2e7dccc9c597a846cf58a973ee13
This was motivated by two things:
1. Notice the bad class name in UserPreferencesChangeHandlerTest. This
was possible because the default mock builder does not check if a class
exists, *and* there was no type hint in place. createMock() *does* check
if a class exists.
2. We are free to use createMock() from PHPUnit 5+ now. It's not only
shorter but more strict and reliable.
Change-Id: I301a171d587026eab0a62575ab2fdbfd7733c661
This is done in preparation for the main patch I8920565. This patch
here is not supposed to change anything. Tests should prove this.
Bug: T233813
Change-Id: I0bdc35dc8cb7bca643f45734b30e374f26b24ad2
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
I believe these additional newlines all make the code easier to read.
It's easier to see what belongs together, and what is a separate thing.
I found the Squiz.WhiteSpace.FunctionSpacing sniff very helpful to
enforce this code style. We enabled this already in almost all WMDE
codebases. It is not yet part of the upstream MediaWiki rule set, but
discussed.
Change-Id: Ibdf788529b28637bf98e7940c2516852c3afcef7
* Explain the difference between the two hooks that both set config vars.
* Remove by-ref & that are not needed. This is a non-breaking change.
Even if the code calling a hook handler provides a variable by reference,
the hook handler being called does not need to require a reference.
Removing these & makes the code less confusing and easier to read.
* Replace an OutputPage type hint with a more narrow IContextSource.
That's all this code actually needs to know. It doesn't need access to
the entire OutputPage, so it doesn't need to require it.
* Update the signature of the ResourceLoaderGetConfigVars handler to
match the caller.
Bug: T215896
Change-Id: Ie1e4b50cd954812f71dd628003b8e9d40fdf5fe8
-> Objects by default as passed by reference so removed & in front
method header.
In addition, updated the PHPDoc comments to reflect the changes.
Change-Id: I69ad2090205f6bd694744779d2c46a3506c85378
As suggested in I69ad209.
As far as I can tell the idea was to be able to pass logger-specific
configuration to the logger factory, e.g. to be able to toggle certain
things the loggers would do. But this is not used at the moment. There
is not much value in keeping unused code around. It can esaily be
introduced again later when it turns out it is needed.
Furthermore, I'm told most of the logging functionality should be
removed anyway. See T193051.
Change-Id: I6b1ddb2a65eacc0e096f2ba44922d63e63212a65
Our documentation generator Doxygen is not able to understand @var tags
on constants. A constant is neither a "variable", nor is it possibly to
specify an allowed type for a constant. The type of a constant is strictly
derived from it's value. There is no way to change it, other than changing
the constant itself. There is not much value in repeating the type either.
If it's critical to know the type of a constant (which is a very rare
situation), that should be better explained in the comment.
Doxygen strips all @… tags it does not understand, and renders everything
else as a plain string. This makes it possible to use something like:
/**
* @const Description…
*/
But this is not different from:
/**
* Description…
*/
Change-Id: I4531d4f6b2ab2aca7a62f13f10a655f31c4d6a7a
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 does make generic `array` type hints more specific when possible.
I'm also applying my personal best practice to not have any @return
documentation on test @dataProviders. These don't provide any useful
information, and can't. The best type we could use is `@return array[]`,
but that would be the same for every single data provider. Copy pasting
these comments around is of no real value.
Also it was already inconsistent as some did not had this comment.
Change-Id: Id401c7e32493b6a9faaf6d47cddc01e2227102af
This is a prerequisit for the later patch Ie0ccb03.
Any JavaScript code can check this feature flag via
mw.config.get( 'wgPopupsReferencePreviews' )
Bug: T213415
Change-Id: I17687c62cc8d738a4eb41738c9ce6662a5ec68d8
This makes the following code work much better in IDEs, e.g. it's now
possible to click method names and actually go there.
Change-Id: Iddbe838f1402c5017ffeb3bc03b4a017b05d8b02
The code literally explains itself. The comments don't add anything
to this. They are more distracting because one must read them first
to understand they don't contain anything.
Change-Id: I6f152962ec634ae15d2bff4472e332453cb9b0bf
I figured a single "@covers ::__construct" in PopupsContextTest should
be enough, especially since this constructor is super trivial.
There is nothing to test in NullLogger. I also removed the duplicated
documentation as it does not really apply, and is still available in the
base class.
Change-Id: I9a2e4b06e5bfd015efa4a92ba802c942290ec49d
