Whe user moves mouse away and we abort the http request we shouldn't
count that request as a FETCH_FAILED. The reasoning behind is that
FETCH_FAILED state increments the counter.PagePreviewsApiFailure.
Our StatsD graph gets polluted with lots of aborted requests and it
becomes unsuable. It doesn't show only the failed requests.
Changes:
- introduced new state: FETCH_ABORTED
- switch to FETCH_ABORTED when browser aborts the request
Bug: T199482
Change-Id: I58047eb80f0700b78b2991daff9395ecc92553b8
Update the mw-node-qunit require to @wikimedia/mw-node-qunit. 2d150f0
missed this and it caused tests in CI to silently succeed.
Bug: T197251
Change-Id: I9de597b0e9afc747c47bddc6debcbe5b87bcd793
* Force arrow-parens
* Disable no-prototype-builtins for time being
* Drop unnecessary maxlen rule
Change-Id: Iceb0fe47354a5753202d2c6ad9e1a9c76791f744
Previous implementation did not pass the `result` variable
to the catch() statement. Because of that every execution that
ended with exception inside fetch() statement was threated as
not a network exception and tried to present the null preview.
Changes:
- properly handle data returned by rejected fetch promise
- chaged the big if (result && result....) into something easier
to read
- pass Error object instead of 'http' string
- Restbase can return exception, it doesn't have to handle the 404
errors by itself, it's already taken care in the catch() logic
- fixed unit tests to reflect new logic in restbase gateway
Bug: T199482
Change-Id: Ibb30fc58248623d9ad4c5388a5b2ff9b387e01de
Instead of mixing window.mediaWiki / mediaWiki and window.jQuery /
jQuery references, always refer to globals which exist whether code is
executed in browser or headless Node.js environments.
find src tests -iname \*.js|
xargs -rd\\n sed -ri 's%window.(mediaWiki|jQuery)%\1%gi'
Change-Id: I21d0a602dcbd2bc6774934bee6c487e443270fe0
Changes:
- added acceptLanguage as a config option passed to
both mwApi and restbaseApi, by default code will use
the language defined in `wgContentLanguage` config
variable. The `wgContentLanguage` is always defined
(see ResourceLoaderStartUpModule::getConfigSettings())
so there is no need for checking the variable existence.
The new logic was tested both on MediaWiki API and Restbase API
Bug: T198619
Change-Id: I1cb31f1999fd674a8b870b2b5effb92ed3dfaa1f
Whenever an HTTP request sequence is started, i.e. wait for the fetch
start time, issue a network request, and return the result, abort the
process if the results are known to no longer be needed. This occurs
when a user has dwelt upon one link and then abandoned it either during
the fetch start wait time or during the fetch network request itself.
This change is accomplished by preserving the pending promises in two
actions, LINK_DWELL and FETCH_START, and whenever the ABANDON_START
action is issued, it now aborts any previously pending XHR-like promise,
called a "AbortPromise" which is just a thenable with an abort() method.
There is a similar concept in Core:
ecc812f06e/resources/src/mediawiki.api/index.js.
Aborting pending requests has big implications for client and server
logging as requests are quickly canceled, especially on slower
connections. These differences can be observed on the network tab of
DevTools and the log in Redux DevTools.
Consider, for instance, the scenario of dwelling upon and quickly
abandoning a single link prior to this patch:
BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_END STATSV_LOGGED ABANDON_END EVENT_LOGGED FETCH_COMPLETE
And after this patch when the fetch timer is canceled (prior to an
actual network request):
BOOT EVENT_LOGGED LINK_DWELL ABANDON_START ABANDON_END EVENT_LOGGED
In the above sequence, FETCH_* and STATSV_LOGGED actions never occur.
And after this patch when the network request itself is canceled:
BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_FAILED STATSV_LOGGED FETCH_COMPLETE ABANDON_END EVENT_LOGGED
FETCH_FAILED occurs intentionally, STATSV_LOGGED and FETCH_COMPLETE
still happen even though the fetch didn't complete successfully, and
FETCH_END doesn't.
Additionally, since less data is transmitted, it's possible that the
timing and success rate of logging will improve on low bandwidth
connections.
Also, this patch tries to revise the JSDocs where possible to support
type checking and fix a call to the missing assert.fail() function in
changeListener.test.js.
Bug: T197700
Change-Id: I9a73b3086fc8fb0edd897a347b5497d5362e20ef
Prevent outdated ESLint error waivers from littering the code by
enabling `--report-unused-disable-directives`.
Change-Id: I3b9c39131f030cf2c4113ecd947c3f4a8679bdfe
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
mw-node-qunit has moved to a new NPM package, @wikimedia/mw-node-qunit.
There are no functional changes with this release but dependencies
should be kept up to date.
https://github.com/wikimedia/mw-node-qunit/blob/9a368a1/CHANGELOG.md#500
Bug: T197251
Change-Id: I25bfc541551cbc29812985df7fa05dc17f0338c5
If a thumbnail is narrow, then the extract can expand to take
the available space. It does this via JavaScript taking the difference
between the normal space for a thumbnail minus the actual space needed
to display the thumbnail.
This removes unused whitespace in both the thumbnail and extract.
Bug: T192928
Change-Id: I59e87f9160e707fbce321a567c0a68e85f6d72ec
Prevents the source_url param in virtual page-views from getting
too long and causing an error because it exceeds varnish's max-url size.
Bug: T196904
Change-Id: Idf3667c4c2ad7e0436f013c70d5ff4ebea453d7a
Make it so the entire popup area is clickable.
Update the click handler to reflect the actual parameter
it receives (an Event not an Element) and do not pass it
in the action, given it is unusedt
Bug: T192773
Change-Id: If80969f4759b1675278d11caaf5cb093ce72031c
Since we use an SVG mask, we cannot use border-left to visually
separate the page preview thumbnail from the text. We can however
make use of a polyline and programatically work out it's start and
end.
Bug: T192928
Change-Id: I0f983a80e3210b2f7e9aa197d2a632680675973e
The tap-dot executable crashes on some test failure outputs.[0] This is
confusing since a test failure in itself often requires debugging. The
issue is present from v1.0.1 to the latest, v2.0.0.
Instead of downgrading, replace tap-dot with the popular
tap-mocha-reporter. This change comes with a bonus: console.log is no
longer filtered out. The benefit cannot be overstated.
[0]: https://github.com/scottcorgan/tap-dot/issues/9
Change-Id: I4ce2d2816885b7c5214f5c1863be595be0d8b1aa
The Popups' JSDocs currently generate without any warnings. Let's keep
it that way for as long as we can by enabling pedantic mode which causes
a failing exit status when warnings are emitted. This behavior can be
verified by adding adding `/** @ignore foo */`.
The JSDoc tag checker should leverage the default enabled dictionaries
but that doesn't seem to be happening[0]. For the time being, allow
unknown tags so that the full range is supported, including @template,
which are very useful for type checking. Minerva already allows unknown
tags.
Lastly, change spaces to tabs since that's what this codebase uses.
[0] https://github.com/jsdoc3/jsdoc/issues/1542
Change-Id: I0aef9f7a6ca4af28d104628cda7763ec70110413
Prevents video files and other non-image files from being rendered as
popup thumbnails. Restricts thumbnail format to either jpg, png, or gif.
Bug: T193792
Change-Id: I7a9be5d1c8396c02ebf0893c960f65644acc9d99
Since it can be unexpected for a pre-commit hook to make edits to the
commit, leverage the existing tests to simply verify that the proper
files have been staged. This also slightly simplifies the existing NPM
scripts and forces the dev to run the same test about to be executed CI,
`npm test` itself, which previously had no other automation tie in.
Change-Id: I74e407ea17a6c2809a49ba56d3ef28b25d5ba5de
The `doc:start` NPM script simply runs the `doc` script whenever a
change is detected. It doesn't seem especially useful for standard
development and adds an extra nodemon depedency. This patch removes the
script and nodemon.
Change-Id: Ib679f6d83bd10f0b8d1572c07080fba7f8d6a701
Replace Istanbul with nyc, Istanbul's CLI. nyc appears to have some
bugs that this patch works around:
- When all files in the project are considered, not just those
imported via tests, the coverage rates and line numbers vary between
runs. This patch disables the `all` option for now and points to the
bug:
https://github.com/istanbuljs/nyc/issues/537#issuecomment-390814662.
- Source map line numbers appear to be incorrect except when `all` is
enabled and working correctly (see previous bullet).
- `sourceMap` must be disabled to avoid ENAMETOOLONG errors when nyc
tries to include them as encoded strings. The patch disables the
setting and points to: https://github.com/istanbuljs/nyc/issues/847.
Using babel-plugin-istanbul and source-map-support appears to have
no effect (the former in tests/node-qunit/run.js and .babelrc).
- CI fails with
`Error: EACCES: permission denied, mkdir '/nonexistent'`. Specify
`SPAWN_WRAP_SHIM_ROOT` instead of constructing a subdirectory from a
nonexistent home directory.
Bug: T196952
Bug: T193519
Change-Id: Idf2e3accd4a6277cbef91c1156fcd206c9e7d882
Via jscodeshift:
jscodeshift \
-t jscodeshift-recipes/src/qunit-assert-equal-to-strictEqual.js \
Popups/tests
Also, some very minor manual clean up.
https://github.com/niedzielski/jscodeshift-recipes/blob/5944e50/src/qunit-assert-equal-to-strictEqual.js
Additional change:
* Drop redundant clipPath parameter from createThumbnailElement - this
parameter does not exist in the function signature.
Change-Id: I209ecf2d54b6f5c17767aa2041d8f11cb368a9b5
The invocation of `rm -rf resources/dist` in package.json
(`check-built-assets`) is replaced with clean-webpack-plugin.
The benefit of this change is that calling `npm run build` now works the
same as the `check-built-assets` script.
Bug: T193522
Change-Id: I64f048855ddceb7159279671b2174a7937e169ff
Changes:
- ignore autocreated flag and always set the popups visilibity state
CentralAuth will create users with `autocreated=on` when user logs
into different wikis
- do not call saveSettings(). AuthManager will call
$user->saveSettings() s after calling all hooks.
Bug: T191888
Change-Id: Iac753ebbaaf175636fdc3e20d8f37883771c33d2
- Adds an extra element in popups template to contain settings icon.
- Resizes the setting icon so that the hover/active state is a square.
- Updates the settings icon SVG file.
- Modifies margin rule for icons placed near top of popup.
Bug: T193058
Change-Id: Icc16a788bba8e2f0a82a27c2b5c7be6c2cccaa90
