Last modified: 2014-02-12 23:52:30 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T49882, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 47882 - mediawiki.page.ready should not be loaded on mobile
mediawiki.page.ready should not be loaded on mobile
Status: RESOLVED FIXED
Product: MobileFrontend
Classification: Unclassified
stable (Other open bugs)
unspecified
All All
: High normal
: ---
Assigned To: Jon
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-04-30 14:59 UTC by Jon
Modified: 2014-02-12 23:52 UTC (History)
10 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Jon 2013-04-30 14:59:03 UTC
why?
It seems to only be needed to run qunit tests but seems to be very unnecessary code for our mobile users.

See Change-Id: I84e0512590de9ff2dbdf519d983a0c1c2d007194
Comment 1 Jon 2013-04-30 15:16:53 UTC
The problem seems to be because the 'mediawiki.tests.qunit.testrunner' module depends on it. This is not a reason to add it to mobile.

Several options here
1) Disable 'mediawiki.page.ready' module on mobile but add target mobile to it in a qunit hook so it works in this mode
2) Rewrite mediawiki.page.ready module to separate out qunit specific code into a separate module and enable that for mobile and replace the dependency to mediawiki.page.ready with that.

Thoughts?
Comment 2 Jon 2013-04-30 15:23:42 UTC
Note to give more background on this module:

the existence of this module on mobile unnecessarily loads jquery.checkboxShiftClick (which allows you to hold shift and click multiple checkboxes ... err how does this work on mobile where there is no shift key?!)

It also adds code to emulate placeholders on certain devices - which might explain a bug in opera mini I've come across where focusing the input shows the placeholder as the search term. This is a really unnecessary on mobile where support is pretty good [2] and placeholder implementations tend to be buggy.

It also runs code that adds accesskey hints to the tooltips. Tooltips are not a mobile thing... o_O

It also loads jquery.makeCollapsible and jquery.tablesorter on certain pages which are probably unnecessary/we can live without.

In summary this module should have never been enabled on the mobile site.

[2] http://caniuse.com/input-placeholder
Comment 3 Juliusz Gonera 2013-04-30 19:20:27 UTC
For now, I'd opt for solution 1 with a bug report in core (mediawiki.tests.qunit.testrunner should not require mediawiki.page.ready) plus a FIXME in our code pointing to that bug.
Comment 4 Gerrit Notification Bot 2013-05-13 22:14:36 UTC
Related URL: https://gerrit.wikimedia.org/r/63584 (Gerrit Change I89d529f0378d90af0fe0a5adea5d5dbdca83a86e)
Comment 5 Gerrit Notification Bot 2013-05-13 22:40:07 UTC
Related URL: https://gerrit.wikimedia.org/r/63589 (Gerrit Change Id79b90204badd82ebd02a9351ca0434c2cb47a21)
Comment 6 Gerrit Notification Bot 2013-05-30 22:28:04 UTC
https://gerrit.wikimedia.org/r/63589 (Gerrit Change Id79b90204badd82ebd02a9351ca0434c2cb47a21) | change APPROVED and MERGED [by jenkins-bot]

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links