Last modified: 2014-02-12 23:54:00 UTC
Created attachment 8991 [details] replacement for MobileFrontend/javascripts/application.js The mobile site js is dependent on jQuery, which pretty much doubles the size of the average page. (Side notes: The current system of attaching events to elements via js after the document loads really doesn't make sense; see comments in attached script. Also, the current setup seems to have the js directly modify the content of the searchfield and title, instead of having them start with the right content. I don't know PHP so I can't help with either of these.)
(In reply to comment #0) > Created attachment 8991 [details] > replacement for MobileFrontend/javascripts/application.js > > The mobile site js is dependent on jQuery, which pretty much doubles the size > of the average page. (Side notes: The current system of attaching events to > elements via js after the document loads really doesn't make sense; see > comments in attached script. Also, the current setup seems to have the js > directly modify the content of the searchfield and title, instead of having > them start with the right content. I don't know PHP so I can't help with either > of these.) The reveal for hash functionality doesn't appear to work correctly.
[#wikimedia-dev] <TrevorParscal>: Krinkle: it's pretty much like this - we need to make the 77 lines of JavaScript code that the mobile site uses ( http://svn.wikimedia.org/viewvc/mediawiki/trunk/extensions/MobileFrontend/javascripts/application.js?view=markup ) to not use jquery Assigning to self
Hm. The revision r1=93490&r2=95684">http://svn.wikimedia.org/viewvc/mediawiki/trunk/extensions/MobileFrontend/javascripts/application.js?r1=93490&r2=95684 seems to have changed what the reveal for hash function does. Previously, when a link led to a header that was within a collapsed section and thus unavailable, the reveal for hash opened up the parent section, making the content available instead of causing the link to be useless. This is also what the reveal for hash does in the submitted change (unless I made a mistake). Now the function seems to open up a section when linked to directly. I'm not sure what the purpose of this is. Is the current revision the intended function, or the previous revision?
(Sorry, above link is broken. r1=93490&r2=95684">http://svn.wikimedia.org/viewvc/mediawiki/trunk/extensions/MobileFrontend/javascripts/application.js?r1=93490&r2=95684 )
(In reply to comment #3) > Hm. The revision > http://svn.wikimedia.org/viewvc/mediawiki/trunk/extensions/MobileFrontend/javascripts/application.js?0=93490&1=95684 > seems to have changed what the reveal for hash function does. Previously, when > a link led to a header that was within a collapsed section and thus > unavailable, the reveal for hash opened up the parent section, making the > content available instead of causing the link to be useless. This is also what > the reveal for hash does in the submitted change (unless I made a mistake). Now > the function seems to open up a section when linked to directly. I'm not sure > what the purpose of this is. Is the current revision the intended function, or > the previous revision? If a link is sent with a fragment identifier like: #See_also it should show that section of the page. The patch that you submitted doesn't seem to do this.
Created attachment 8996 [details] fix Now does both the previous function and the current function.
Before I continue review, what is the intended/required browser support of MobileFrontend ? I see a few problems in the JavaScript that aren't problems in the "perfect browser", but are in IE, Opera or old verisons of Firefox.
Another question I couldn't quite find an answer to right away: Does MobileFrontend disable any and all other potential JavaScript on the page ? (ie. it gets the page content itself and generates the page and the javascript inclusion). If so, verify that none of the following is loaded: * MediaWiki core resources and other modules loaded through ResourceLoader * Gadgets, site scripts (Common.js), user scripts (User:Foo/vector.js) * Extensions that might inject javascript though addScript and the like.
(In reply to comment #7) > Before I continue review, what is the intended/required browser support of > MobileFrontend ? > > I see a few problems in the JavaScript that aren't problems in the "perfect > browser", but are in IE, Opera or old verisons of Firefox. Ideally we would want to support those browsers as well. But, it is mostly geared towards mobile based browsers.
(In reply to comment #8) > Another question I couldn't quite find an answer to right away: > > Does MobileFrontend disable any and all other potential JavaScript on the page > ? It removes the scripts that might have been included via the head section. > (ie. it gets the page content itself and generates the page and the javascript > inclusion). It generates the javascript inclusion. > > If so, verify that none of the following is loaded: > * MediaWiki core resources and other modules loaded through ResourceLoader > * Gadgets, site scripts (Common.js), user scripts (User:Foo/vector.js) > * Extensions that might inject javascript though addScript and the like. They shouldn't be.
(In reply to comment #9) > (In reply to comment #7) > > Before I continue review, what is the intended/required browser support of > > MobileFrontend ? > > > > I see a few problems in the JavaScript that aren't problems in the "perfect > > browser", but are in IE, Opera or old verisons of Firefox. > > Ideally we would want to support those browsers as well. But, it is mostly > geared towards mobile based browsers. I used those names as examples, but of course we're talking about mobile browser here. Sadly, mobile browsers are even 'worse' than desktop browser when it comes to consistency in support of JavaScript APIs. I understand you likely won't have a huge table with support for MobileFrontend yet, since it's still in development and what is supported partially depends on what it actually can work on. (ie. we support as much as possible, and if it happens to work in X, then X could be added to the list of supported mobile devices). But it would helpful in review, if there is at least some kind of list with mobile devices that it must work on. When I started reviewing, one problem came up already. The script tries to read the 'display' property of the 'style' property of the dom element node for the #logo. That works fine in Mobile Safari or Google Chrome, but not in Internet Explorer on pocket-pcs, pdas or phones running Microsoft software with Internet Explorer (up to a certain version). In some versions of internet explorer: var a = document.createElement( 'div' ); a.style.display // does not give 'block' Instead it has to be computed via getComputedStyle. And there are more issues like this, hence the existence of jQuery and other libraries. I'm happy to further review this script, and on first glance it works fine in a 'perfect' browser like Mobile Safari or Google Chrome. But assuming this is not just intended for Android and iPhone, I need to know which browsers/devices it must run on and as such which problems (that are really problems in those browsers) we need to fix.
I didn't know any browser gives 'block' for: var a = document.createElement( 'div' ); a.style.display . The expected result in the script is an empty string. Do you mean that: var a = document.createElement( 'div' ).style; a.display = 'block'; a.display //does not give 'block' ?
(In reply to comment #0) > Created attachment 8991 [details] > replacement for MobileFrontend/javascripts/application.js > > The mobile site js is dependent on jQuery, which pretty much doubles the size > of the average page. (Side notes: The current system of attaching events to > elements via js after the document loads really doesn't make sense; see > comments in attached script. Also, the current setup seems to have the js > directly modify the content of the searchfield and title, instead of having > them start with the right content. I don't know PHP so I can't help with either > of these.) Thanks for the patch and continued work Yair. One thing that were going to have to have community discussion about before we close this bug is wether we will remove jQuery from all devices as this bug states or some bugs as https://bugzilla.wikimedia.org/show_bug.cgi?id=29505 says. There is no question that we want to keep payload size small. But as we start to increase the functionality for mobile we want to start using established frameworks that don't significant'y increase our payload but at the same time we need to allow the ease of development to not have to re-invent functionality and code each time we add a new feature. Having to do custom js work for each mobile feature will eventually cause our payload to grow larger then we want it and it will further complicate our implementation. We really need to not have both happen. In order to do this we'll have to experiment with frameworks like http://zeptojs.com/, http://joapp.com/, http://xuijs.com/, etc to see what will give us the best balance for mobile while keeping compatibility with our PC site using jQuery. We'd love to get your help testing these.
