Use the eslint rule
"no-var": "error"
in resources/.eslintrc.json and in .eslintrc.json
to require the use of `const` and `let` instead of `var`.
Bug: T337102
Change-Id: I3b3d6d426966a97c13f62494443f62bd5b790920
This test was leaving behind the black overlay, obscuring the test
results interface. Upon closer inspection, it appears not one part
of this test is doing anything that actually works.
* Assertion 1: `viewer.isOpen === false`
This is the initial and default state. Not essential for the test,
but so far so good.
* Prep for Assertion 2 & 3:
We call loadImageByTitle. One might think that this will set isOpen
to true, but, this isn't asserted. If one did, the test would fail,
because loadImageByTitle returns early from its first branch
in `!this.thumbs.length` and is thus a no-op, and adds nothing to
the test.
We set `location.hash = 'Foo'`.
* Assertion 2: `location.hash === '#Foo'`
Assertion 3: `viewer.isOpen === false`
Assertion 2 is meaningless because handling of hash changes appears
to be asynchronous. Perhaps not always, but at least the indirection
used in this codebase makes it async. Hence, the fact that it is
correctly normalised to '#Foo' is meaningless as the popstate
and hashchange event handler haven't been processed yet. If they
were, it would still be meaningless since viewer.isOpen was never
true in the first place and MMV's event handlers for popstate/hashchange
are guarded by `isOpen`.
* Prep for Assertion 4: location.hash = Bar
* Assertion 4: `location.hash === '#Bar'`
Idem.
* Prep for Assertion 5: Replace the viewer.loadImageByTitle function
with a function that performs a nested assertion.
This function is never called and its assertion never reached.
This is a textbook example why assertions should never be
nested. If you rewrite this in following with the advice from
<https://api.qunitjs.com/assert/verifySteps/>, we find that
the array is in fact empty by the end of the test() function.
```js
var seen = [];
viewer.loadImageByTitle = function ( title ) {
// assert.strictEqual( … );
seen.push( title.getPrefixedText() );
};
location.hash = …;
location.hash = …;
location.hash = …;
location.hash = …;
assert.deepEqual( seen, [ 'File:' + imageSrc' } );
// Actual: []
```
== Misc notes ==
The test ends with "#/media/File:Foo bar.jpg" set on location.hash,
and the black overlay covering the page. It seems that the hashchange
are async, and that the first in a given event loop tick "wins", with
later ones being ignored. Observing this with debugger/breakpoints is
hard given that each pause results in the effect not being observable.
console.log(location.href) at the end of the test shows hash='#',
but placing a debugger at the end of the test results in the hash
being "#/media/…" instead.
Exprienced in Firefox 117 on macOS.
Change-Id: Ib37bec1b3e67fcab1da89d23381a3adbb6312827
This makes the test report inaccessible in the browser. I'm adding
it at the module level given that nearly all test cases in this
file trigger it:
* "Progress"
* "Progress when switching images"
* "New image loaded while another one is loading"
* "Events are not trapped after the viewer is closed"
* "document.title"
```
- QUnit.module( 'mmv', QUnit.newMwEnvironment() );
+ QUnit.module( 'mmv', QUnit.newMwEnvironment( {
+ beforeEach: function () {
+ this.sandbox.stub( require( 'mediawiki.router' ), 'back', function () {
+ console.log( QUnit.config.current.testName );
+ console.trace();
+ } );
+ }
+ } ) );
```
Change-Id: I0cb7ed6c76d5547bcef9c183c9305b6fd08ec9eb
- mediawiki.router is available inside mmv.bootstrap so move
the handlers into the bootstrap file
- Remove direct loading of images from mmv.js instead modify
event handlers to modify the location which will trigger the loading of
the image
- Copy what we previously did in MobileFrontend (Bartosz's idea)
to allow page refreshing and escaping overlay on page load.
- Remove some no longer relevant tests
Bug: T236591
Change-Id: Ie5d33fca5174db58380a63f5cf97f44e5df10d42
The
progressBar: {
hide: this.sandbox.stub()
}
fixes the following exception in the JavaScript console:
jQuery.Deferred exception: progressBar.hide is not a function MMVP.setupProgressBar
The
router: {
checkRoute: function () {}
}
fixes the following exception in the JavaScript console:
jQuery.Deferred exception: viewer.router is undefined
Change-Id: Ia7658937ebdbb77a5889d6462f7706bb2fd4e08f
When an svg file is requested, returns the 1024px file and displays at
max possible size
Bug: T71237
Change-Id: I797794d1f9b1aa66cbb9bd9fec57274554292fb1
* Removed deprecated testCount, which is no longer supported in this way.
It's only useful when wanting to explicitly expect 0 assertions (normally
considered an error).
* Replaced use of testCount = 0, with assert.expect( 0 ).
Change-Id: I8fe82032d3af2a1ad2ed7febdb90a95d56dd3fc4
This heavily relied on deferreds getting resolved synchroneously,
which (for .then) is no longer the case with jQuery 3.
There's also a difference in how chained .then's get resolved,
and $.when no longer propagates notify().
The changes in here are basically:
* fix use of $.when, manually passing along notify()
* use .then in some places, instead of .done, .fail, .progress
* fix progress bar hiding in setupProgressBar, which assumed sync
* fixed tests, mostly by using fake timers to give async stuff a
chance to execute
Bug: T164473
Change-Id: Ib51ddd8bc6254a477861588fb16f57565353afe1
Some of the code encountered accessed mw.config directly,
I cleaned that up by migrating to mmv.Config, which is an
abstraction added to avoid peeking at mw.config directly.
Bug: T132064
Change-Id: I2a95ba703e6c7f46296f8e435bceec769dceebf9
Having many modules makes the startup module in the head
unnecessarily longer. Things should only be in their own
module if they're going to be loaded on demand on their
own or are a shared dependency of separate modules that
are.
This change brings down the amount of modules declared by
Media Viewer from 53 to 8. The ones that remain are
mostly things loaded on demand.
Bug: T103706
Change-Id: I5b0d29209599285b93217e96def818e641646c73
Pass alt parameter from mmv.bootstrap.js to mmv.js and
set it as a parameter on the displayed lightbox image.
Include the alt text in the embed text.
Bug: T66519
Bug: T75923
Change-Id: I29503eb582ac2bc8cf89f737a3bcb787b660d918
This is complete, but it would be better if the HEAD request
was actually aborted by Varnish when the viewDuration parameter is
present, or if the hit pointed to a script that does that.
Change-Id: I66cafd97427756411e967de1901324af2215e3ae
Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/1001
Changes document title (which is shown in history navigation)
to include the image name.
Bug: 67008
Change-Id: Id1b030f2b984571fb0877e35db2ca2ccc86f0130
Per the design meeting, we are abandoning this feature for now - there is not enough time to ensure it is of acceptable quality.
This reverts commit 4329d453ec.
Change-Id: I27c113ffecb617d442557163722ea5181ed0b2f4
Just a link to the full-size file for now.
Since the link must be to a PNG/JPEG/GIF (so possibly a thumbnail),
and we want to cap the size, we might need to get the URL from the
API, but we need to open the new window right away to avoid popup
blockers, making this patch quite complicated.
Change-Id: I9ce9d2a2d27b75470eae2806d9f9ce2f95f4dac2
Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/588
This tries to fix a number of related issues:
* the blurred thumbnail was visible for a split-second sometimes
when switching back to an already-loaded image. (Presumably when
JS was sluggish enough to take more than 10 ms to execute.) We
now check whether the promise is pending before showing a placeholder.
(More generally, a lot of unnecessary logic was executed when paging
through already loaded images, like displaying the placeholder, so
this might make the UI a bit more responsive.)
* the blur could get stuck sometimes - I have seen this a few times,
but have never been able to reproduce it, so I'm only guessing, but
maybe the timing was really unfortunate, and we switched back less
than 10 ms before loading finished. We now remove the blur on every
branch, just to be sure.
* adding a progress handler to a promise might not have any immediate
effect, so when switching to an image which was loading, the progress
bar reacted too late. We now store the progress state per thumbnail
so it is always available immediately.
* the progress would animate from 0 to its actual state whenever we
navigated to the image. The change on paging is now instant; the
progress bar only animates when we are looking at it.
* switching quickly back and forthe between a loaded and a loading
image resulted in the loading image becoming unblurred. This seems
fixed now, I'm not sure why. Maybe the "skip on non-pending promise"
logic affects it somehow.
Also removes some unused things / renames some things which were
confusing, and makes an unrelated fix in the image provider, which kept
amassing fail handlers.
Change-Id: I580becff246f197ec1bc65e82acd422620e35578
Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/489
Solves the bug, and makes the code slightly cleaner, but it
still does not inspire confidence (e.g. use of viewer flags
by a bunch of callbacks that can run for a background image).
Also, the tests seem underspecified.
I'll follow up with some more refactoring.
Change-Id: I2557abcec173691ffce21185bf1a939f1644ba8c
Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/489
* deduplicates URL generating/parsing code
* gets rid of spaces in URLs
* fixes error for file names with / in them (in case they exist;
current MediaWiki seems to disallow such names anyway)
Change-Id: I5aad43f6af1b99523c597c39befcc9db1ecab83a
Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/371
