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
We discovered a bunch of possible solutions (see previous patch sets),
including replacing the `$( document )` selector with a more specific
one. That idea does not pass the linter.
Very late I realized the original selector starts with
`#mw-content-text`. This heavily limits where popups are allowed to
appear: really only in the main text content area.
We should limit reference popups to the exact same scope.
This fixes the issue described in T215195. Before, the content of the
popup was covered by the selector. Reference links *inside* the popup
would trigger another popup, which makes the current popup disappear.
Now the popup itself is not covered by these event handlers any more.
Bug: T215195
Change-Id: I142aee68abbd57ca321873855fef9209e0db0bbf
This is mainly done to increase testabilty of this part. I am a bit
unsure if this should ( have been ) integrated in the former index.js
that's now the page.js. - See also the refactoring done before.
Bug: T214971
Change-Id: I90d0441510bc1ec0b4900a392afcbaff6a552377
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
This adds support for preview popups on reference/footnotes from
the Cite extension. For that a new preview type was introduced and
integrated into the existing structures.
The essential starting points were this code comes into action are
added behind the feature flag introduced in the previous patches.
Bug: T213415
Change-Id: Ie0ccb03117bd654373d0f458b62cc52018361c67
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 became an issue after the patch Ifa56d41 (part of T206323) removed
some hidden <span> and replaced them with title="…" tags. The CSS selector
in src/index.js get's active on all <a> elements that have a title="…"
attribute. This is now the case for all references that are used one time
and have their ↑ (or ^ on the English Wikipedia) linked.
I realized this is really only an issue for these ↑ links.
The more general issue described in T198652 still holds true, but becomes
less urgent with this bugfix.
Bug: T198652
Bug: T206323
Bug: T212419
Change-Id: I9287e8692d031f9d2ba50f967520bf327ed5c42f
Replace $.noop dependency with inline empty function.
The tests relied on a single function instance to pass. A toString()
comparison of the noop function cannot be used as they differ when
coverage is enabled.
Change-Id: I641801593beb240a8f7d06e388a0e41dc8a25bc6
Currently the Popups schema is disabled but if we were to re-enable
it, the page tokens would now be consistent with the ReadingDepth
schema
The getToken function is kept to allow generation of the
pageInteractionToken's relating to a link interaction, for which
there is no centralised API.
Additional changes:
* I've updated various tests to use the central mw.user stub to make
clearer where tokens come from.
Bug: T203013
Change-Id: If746bea5aeed2b4c192a9b8a02feb1fe06480633
Let's improve our documentation by linting it and ensuring it
is complete and matches guidelines
This fixes offenders
Change-Id: I7c829b375705e763085cf731e9a77cc14339af67
Although Popups only uses JSDocs at this time which seemingly doesn't
care about casing[1], we should endeavor to use the proper return types.
This patch lowercases typing to indicate primitive / boxed type as
appropriate.[2] As a special case, function types are uppercased for
compatibility with TypeScript type checking.
Lastly, JQuery types are of type "JQuery". The global JQuery object's
identifier is "jQuery". This patch uppercases J's where appropriate.
[0] https://github.com/jsdoc3/jsdoc/issues/1046#issuecomment-126477791
[1] find src tests -iname \*.js|
xargs -rd\\n sed -ri '
s%\{\s*([?!])?(number|string|boolean|null|undefined)%{\1\L\2%gi;
s%\{\s*([?!])?(function|object)%{\1\u\2%gi;
s%\{\s*([?!])?jquery%{\1JQuery%gi
'
Change-Id: I771bdbb69dc978796a331998c0657622ac39c449
Redux DevTools are available in all builds by passing the `?debug=true`
query string. Since globally enabling debug significantly slows load
times, also enable support when the build is non-production (debug)
which is known at transpile time. This enables a debuggable version of
Popups in an otherwise production-like MediaWiki without changing the
Popups release build product.
Also, update the readme with a couple debug tips and flip a few bullets
from hyphens to asterisks since that seems to be more prevalent.
Change-Id: I4cab0b8069b12505dbfa840939caac196bae2750
No longer needed. We can't turn something off again for people
now they expect it to exist.
Clarified usage instructions of PopupsEventLogging to make sure
it's more scary given the implications
Bug: T173952
Change-Id: I7be005b79da498d8e7b7df8f18b60c1327636a2c
Popups is out of beta feature and this code is no longer needed.
Removing code is the happiest activity a developer can do.
Other changes:
* Remove redundant type field on extension.json
(If not set, the extension will default to the "other" section.)
* Repurpose `name` with `namemsg` and make use of existing i18n
messages
Bug: T193053
Change-Id: Iea832cd1f37b0e7df6ff95efd66e4a1ff2a9004e
We already request an idle callback when loading the code in the init
module.
Doing so here seems redundant.
Bug: T191089
Change-Id: If132c3331c49a4e74be70a5486a8270a8ce380bd
...by using a delegate event handler on the document instead of waiting
in the hook for the content div.
Bug: T191089
Change-Id: I88baa12a9aad9ed5e6c1288b39089843c19cec6c
We are using EventLogging to track page views not user behaviour.
This is an exception to the rule and requires special handling.
Bug: T190188
Change-Id: If096ccaf0ac884d57744ed57e2f26b51446de2d7
Comment became a bit stale. This updates it to reflect reality and
normalizes the verb forms for consistency.
Noticed as part of https://gerrit.wikimedia.org/r/c/420839/2/src/index.js#167
Additional changes:
* Merged all the const statements in L63 into one, for consistency.
Before, some were merged and some were independent.
Change-Id: I23aa824bb811f03a3630b4695e84c468bd9cd8b5
Specifically, clarify why the timestamp property is merged into the data
bag that represents the event when it is.
Change-Id: I184de734e66490fc728a4f9c2f84bf0765aeed08
This change updates the schema and begins to log
additional information such as source_namespace, id
and title. This information is provided inside the
boot action.
Additional changes:
* Allow camel case in bundle artifact
Bug: T184793
Bug: T186728
Change-Id: I425ffecc018bef2958d0dfe957a40a065e3e6c56
Pageview is consistent with verbiage used by Research and Analytics
Engineering in their reports and documentation, e.g.
https://wikitech.wikimedia.org/wiki/Analytics/Pageviews.
Bug: T184793
Change-Id: I8ae085b4af85aa72f234f3db27f0cac2c4d014e5
* New action added PREVIEW_SEEN
* The action will be used to signal that a page view needs
to be recorded.
* PREVIEW_SEEN is a delayed action which is triggered
as a side-effect of the previewShow action. It is only dispatched
if the user is still previewing the same card and the page
related to the card has preview type `page`
* The pageview changelistener is added when
$wgPopupsVirtualPageViews is set to true.
* The page view changelistener listens for page views and logs
them using EventLogging when needed using
https://meta.wikimedia.org/wiki/Schema:VirtualPageView
Note:
* Currently if a user has enabled the DNT header, the
event will not be logged. There is ongoing discussion on the
ticket and fixing this will be addressed separately.
* Only title and referrer are logged in the initial version.
The task demands that "namespace" is logged but this information
is not provided by the summary endpoints we use so will need
to be added later (if indeed needed) either via a change to that
endpoint of by using JavaScript to parse the URL.
Bug: T184793
Change-Id: Id1fe34e4bdada3a41f0d888a753af366d4756590
Enforce it with eslint.
Ignore:
* Comment lines with eslint disable directives
* QUnit test lines as they contain long subjects (QUnit.* (only, test,
module, skip, etc)
* Strings, since long strings are used extensively in tests
* Ignore template literals for similar reasons
* Regex literals as they may be too long, but can't be easily
split in several lines
* Long urls
See bug for more general proposal for eslint-wikimedia-config.
Bug: T185295
Change-Id: I3aacaf46e61a4d96547c513073e179ef997deb09
Event timestamp (as reported by `window.performance.now()`) is also sent
along with every action.
Bug: T180036
Change-Id: Ie3a648298005d51b8b4fbaa6a53e78caf50fa2d3
Introduce PopupsAnonsExperimentalGroupSize config variable. This defines
a population size who will be subject to experimentation. If the group
size is undefined or 0 (default) and PopupsBetaFeature is false
(default value) Popups will be enabled for everyone. If it is any other
value, half that group will see page previews.
Drop the config variable PopupsSchemaSamplingRate - we will now only
EventLog when an experiment is occuring. This means we can simplify the
MWEventLogger class as shouldLog will always be truthy. Given server
side eventlogging is only used for preference changes
traffic should be low and not need sampling.
Introduce getUserBucket which determines whether a user is in a bucket
on, off or control based on the value of
PopupsAnonsExperimentalGroupSize. Add tests showing how these
buckets are calculated.
Caution:
A kill switch wgPopupsEventLogging is provided for safety.
It defaults to false. Before merging, please check if any config changes
are necessary.
Bug: T171853
Change-Id: If2a0c5fceae78262c44cb522af38a925cc5919d3
We haven't seen the PP EventLogging instrumentation produce duplicate events
for weeks.
Bug: T172106
Change-Id: I6f3d7c0cdbf23161f63259e4d20d8a710376468b
Instead of registering global variables in a function, make it pure
return the external interface and set it to mw.popups in the
src/index.js entry point.
Explicitly comment on index.js what is being set and why.
Bug: T171287
Change-Id: I94d467bfa7fa6e56033dd254518ad50b5dde5bfc
Changes:
- introduced js module defining mw.popups object
- introduced isEnabled() method which checks the redux store to retrieve
isEnabled status
Bug: T171287
Change-Id: I523369831e2aa8a915ed1cb001b35d13b770f9da
Why: Because they are the approved standard by TC39 and Ecma for
JavaScript modules.
Changes:
* Wrap mw-node-qunit in run.js to register babel to transpile modules
for node v6
* Change all sources in src/ to use ES modules
* Change constants.js to be able to run without
jQuery.bracketedDevicePixelRatio given ES modules are hoisted to
the top by spec so we can't patch globals before importing it
* Change all tests in tests/node-qunit/ to use ES modules
* Drop usage of mock-require given ES modules are easy to stub with
sinon
Additional changes:
* Rename tests/node-qunit/renderer.js to renderer.test.js to follow
the convention of all the other files
* Make npm run test:node run only .test.js test files so that it
doesn't run the stubs.js or run.js file.
Bug: T171951
Change-Id: I17a0b76041d5e2fd18e2d54950d9d7c0db99a941
Following on from I4f653bba, since the schema and statsvInstrumentation
modules are similar, let's group/rename them:
schema -> instrumentation/eventLogging
statsvInstrumentation -> instrumentation/statsv
Change-Id: Ic59e0da7d4917f6733fd090f15d3c269af863f05
Currently, the mw.eventLog.Schema class samples per pageview. However,
we expect that if a user is bucketed for a session, then all
EventLogging events logged during that session are in the sample.
Moreover, loading the class in the way that we did - asynchronously,
using mw.loader#using - introduced an issue where the eventLogging
change listener would subscribe in the next tick of the JavaScript VM's
event loop and miss the "pageLoaded" event being queued (see T167273).
Changes:
* Make the schema module follow the form of the statsvInstrumentation
module, i.e. make it expose the #isEnabled method, and add the
associated getEventLoggingTracker function.
* Update the eventLogging change listener accept the tracker returned by
getEventLoggingTracker.
* Update/fix related JSDoc documentation.
Bug: T167236
Bug: T167273
Change-Id: I4f653bbaf1bbc2c2f70327e338080e17cd3443d4
And add tests, given it is growing in complexity.
Additional changes:
* Interface ext.popups.Gateway -> Gateway in docs
Bug: T165018
Change-Id: I8a12333ad9d14d6a7fbde11afc42f607881e8ea3
... from the statsvInstrumentation module so that the bucketing logic
can be shared with other instrumentation modules.
Change-Id: I5732fa539a3911939fa85fa88c102fa8dcfa5613
Refactor existing Restbase gateway and extract shared logic into
shared Restbase provider. Also introduced new createNullModel()
which defines an empty preview model.
Additionally improve naming in new gateways/formatter so function
names are more explicity.
* Htmlize() became formatPlainTextExtract() as it should be used
only with plain text extracts
* removeEllipsis() became removeTrailingEllipsis() as it removes
only trailing ellipsis.
* src/gateway/index.js defines gateways by configuration name stored
in extension.json
Bug: T165018
Change-Id: Ibe54dddfc1080e94814d1562d41e85cb6b43bfc1
Depends-On: I4f42c61b155a37c5dd42bc40034583865abd5d7a
...instead of 1 event per link
Supporting changes:
* Delegate events on the container when booting up
* Check eligibility of title on event triggered
* Pass the title from the event handlers into the actions instead of
storing it in the dom
* Add title.fromElement as sugar over isvalid(gettitle())
* Not tested as it is sugar over the other 2 functions
* Fix action tests and integration tests
processLinks to be removed next
Bug: T165572
Change-Id: I4d9837706dc77ec64121ac94410c0d2da83692e4
This ensures all files belong to a module.
When generating documentation no globals should be present
Bug: T158236
Change-Id: I134f38620fe46db11ba94dbede739f4336e0482c
The eventLogging change listener is responsible for ensuring that the
internal state of Page Previews matches its external state (that
perceived by the user and UA). It does this by logging events with
ext.eventLog.Schema#log. This makes it the perfect place to track and
discard duplicate events enqueued by the Page Previews codebase observed
in T161769.
Make the change listener track events that it's logged by storing hashes
of the dynamic parts of them in memory. If the eventLogging change
listener sees the same event more than once, then it discards it and
increments a counter in StatsD.
This behaviour should be enabled for a matter of days as we should see
whether the duplicate events are being enqueued by the Page Previews
codebase immediately.
Bug: T163198
Change-Id: I6a38a2619d777a76dd45eb7300079e1f07b07b12
The statsv change listener depended on both the analytics tracking
function and whether it should log metrics to StatsD. We can simplify
the behaviour of the change listener by passing in a function which
doesn't log metrics to StatsD if such logging is disabled.
The change listener is now more isolated from other components.
Moreover, sharing the analytics tracking function with other components
is simpler as there's no repeated code.
Bug: T163198
Change-Id: Ibf4785fa4c27c1ad4739f02410f57412f56ff481
