MediaWiki talk:Common.js/Archive 17: Difference between revisions

Revision as of 08:28, 27 February 2010

Revision as of 08:30, 3 March 2010

Line 672:

::But we still use the javascript for sysops and accountcreators shown in the first code box above.

::--[[User:Davidgothberg|David Göthberg]] ([[User talk:Davidgothberg|talk]]) 12:11, 14 January 2010 (UTC)

== WikiMiniAtlas ==

I have noticed that [[meta:MediaWiki:Wikiminiatlas.js]] is loaded on every page view, no matter if a page has coordinates or not. I intend to make it so it only loads when needed. (All our other special scripts are only loaded when needed.)

Loading the WikiMiniAtlas code costs load time on the first page view, and it costs rendering time on every page view. (Some users still have slow connections and/or slow computers.) And it costs bandwidth for the Wikimedia servers to transfer that file. Last time I checked the stats the average visitor to Wikipedia only views five pages in one session, which means that many visitors don't view any pages with coordinates.

As far as I can see all coordinates here at the English Wikipedia are added by using the {{tl|coord}} template. That template renders very differently in different situations, using different classes and ids. So I will add a CSS id we can trigger on at the beginning of that template, like this:

:<code>&lt;span id="wikiminiatlas-load">&lt;/span></code>

And I will change the code in [[MediaWiki:Common.js]] that loads [[meta:MediaWiki:Wikiminiatlas.js]] to this:

<source lang="javascript">

addOnloadHook( function() {

var wmaload = document.getElementById("wikiminiatlas-load");

if ( !wmaload ) return; //The page has no coordinates.

if (wgServer == "https://secure.wikimedia.org") {

var metaBase = "https://secure.wikimedia.org/wikipedia/meta";

} else {

var metaBase = "http://meta.wikimedia.org";

}

importScriptURI(metaBase+"/w/index.php?title=MediaWiki:Wikiminiatlas.js&action=raw&ctype=text/javascript&smaxage=21600&maxage=86400")

} );

</source>

I will notify the people who work with coordinates and Dschwen who seems to be the main coder of the WikiMiniAtlas system.

--[[User:Davidgothberg|David Göthberg]] ([[User talk:Davidgothberg|talk]]) 17:21, 15 January 2010 (UTC)

:Wikiminiatlas is currently active for every geolink {{tl|coor URL}}. That is every link that starts as http://stable.toolserver.org/geohack/geohack.php. I'm not sure we would want to complicate this system even further. Also, I note that there can be multiple coords on one page, so adding a unique id is not valid in that case. Adding a class makes the process more complicated however, because the getElementsByClassName (which is really slow on some browsers) will have to be run twice. —[[User:TheDJ|Th<span style="color: green">e</span>DJ]] ([[User talk:TheDJ|talk]] • [[Special:Contributions/TheDJ|contribs]]) 17:29, 15 January 2010 (UTC)

:Agree with TheDJ here. As outlined abobe this is not a good approach. Let me think about it. --[[User:Dschwen|Dschwen]] 18:13, 15 January 2010 (UTC)

::I am aware that there can be multiple coordinates on the same page. But adding the same id several times on a page is no problem since I only use it to trigger the loading, not as an anchor. And using an id is of course the best option since checking it is efficient.

::TheDJ: Thanks for pointing out {{tl|coor URL}}. It seems the trigger id should be added to that template instead.

::--[[User:Davidgothberg|David Göthberg]] ([[User talk:Davidgothberg|talk]]) 18:30, 15 January 2010 (UTC)

:::It's not "no problem", it's ''invalid HTML''. Both [http://www.w3.org/TR/html401/struct/global.html#adef-id HTML4] and [http://dev.w3.org/html5/spec/Overview.html#the-id-attribute HTML5] require absolutely explicitly that the id value must be unique within the document. I assume you want to use an id instead of a class so you can use <tt>getElementById()</tt> rather than the more expensive <tt>getElementsByClassName()</tt>; tough. You can't break HTML validity just for the sake of a few miliseconds of JS execution time. I wouldn't be surprised if a lot of JS, Twinkle especially, broke entirely. [[User:Happy-melon|<span style="color:forestgreen">'''Happy'''</span>]]‑[[User talk:Happy-melon|<span style="color:darkorange">'''melon'''</span>]] 18:51, 15 January 2010 (UTC)

::::My concern was also particularly for Twinkle. We've seen it stumble over broken HTML quite a few times already. —[[User:TheDJ|Th<span style="color: green">e</span>DJ]] ([[User talk:TheDJ|talk]] • [[Special:Contributions/TheDJ|contribs]]) 19:45, 15 January 2010 (UTC)

::TheDJ and Happy-melon: As far as I know all web browsers handle multiple instances of ids nicely. And multiple ids are commonplace since it is such a common "mistake", so browsers must handle it. And it is commonplace here at Wikipedia too, since some templates that were originally meant to only be used once, and have an id, over time have come to be used more than once on some pages. And pages sometimes have the same section title or shortcut/anchor more than once on a page, which means the same ids are repeated. If Twinkle chokes on ids, then it is seriously broken and should be fixed.

::Happy-melon: Running getElementsByClassName() isn't about milliseconds. On older computers we are talking many seconds, on every page load. Since the "Usability" project uses so inefficient code, among other things with several getElementsByClassName() calls, there now are pages here at Wikipedia that takes about one minute to load on older computers. I have such an old computer, I have had to adblock the "Usability" code in my browser to be able to continue to use Wikipedia.

::That millions of Wikipedia users should unnecessary spend extra time to load pages because of the WikiMiniAtlas code is bad and should be fixed. Using an id is the simplest and most efficient way to fix it for pages that don't need the WikiMiniAtlas code.

::But if you guys are so worried about ids, then I suggest you ask the devs to add a magic word that lets us add an id (or perhaps even other items) at multiple places in a page, but only renders the first of those items on the page.

::--[[User:Davidgothberg|David Göthberg]] ([[User talk:Davidgothberg|talk]]) 21:23, 15 January 2010 (UTC)

:::At closer inspection, it doesn't even use getElementsByClassName. It just gets all the links on the page (which can be a lot of course). —[[User:TheDJ|Th<span style="color: green">e</span>DJ]] ([[User talk:TheDJ|talk]] • [[Special:Contributions/TheDJ|contribs]]) 22:43, 15 January 2010 (UTC)

::::What if we set a maximum allowed execution time on the script .... ? We could bail parsing A elements after 10 seconds for instance ? It would only handle as many coordinate links as it has been able to parse in that time. —[[User:TheDJ|Th<span style="color: green">e</span>DJ]] ([[User talk:TheDJ|talk]] • [[Special:Contributions/TheDJ|contribs]]) 22:45, 15 January 2010 (UTC)

:::DG: Saying "it's ok because web browsers seem to be alright with it" is like saying it's ok to use malformed HTML because we know Tidy cleans up after us: we don't jump through fire hoops to eliminate it because we have that safety net, but it's ''not'' just "ok" because we know that it has the potential to cause serious problems. You're normally a champion of not using outlandish hacks when they result in unstable code, I'm honestly surprised to see you so happy to do so here.

:::Can someone say whether the hasClass() function in our JS is genuinely significantly faster than the native getElementsByClassName(), and if so, is it possible to work the efficiency increase into the site version? I'll happily commit an improved native function if someone wants to write one. [[User:Happy-melon|<span style="color:forestgreen">'''Happy'''</span>]]‑[[User talk:Happy-melon|<span style="color:darkorange">'''melon'''</span>]] 23:11, 15 January 2010 (UTC)

:TheDJ: I didn't say that the WikiMiniAtlas code is using getElementsByClassName(). What I said is that we shouldn't use that function to decide whether to load the WikiMiniAtlas code or not, since that function is to slow.

:Happy-melon: Using CSS ids is not an outlandish hack, instead it is a well working tool. That ids should just be used once is just the typical impossible to honour theoretical demand from the W3C people. They have many such weird demands that most web sites can't bother about. Wikipedia breaks the W3C standards in many places, since reality goes before theory. (But there is one W3C recommendation that web browsers do honour: They do graceful degrade.)

:Note that Wikipedia pages now take 30-60 seconds to load on older computers. That's what this is about. Reading Wikipedia with a slow computer is now only barely acceptable. And it is no longer possible to edit Wikipedia if you have a slow computer, unless you have exceptional patience or know how to apply a whole bunch of blocks and fixes to make Wikipedia load faster. This might be one of the reasons why the number of editors is declining. (It is well known that many of our editors are unemployed, that's why they have time to edit, and unemployed people often can't afford to buy a new computer.) And we want Wikipedia to be used also in less developed countries.

:We need to do anything we can to make Wikipedia useful for those that can't afford to buy the latest computers.

:--[[User:Davidgothberg|David Göthberg]] ([[User talk:Davidgothberg|talk]]) 06:33, 16 January 2010 (UTC)

=== WMA loader rewrite ===

Looking at the WMA code, it is not as efficient as it could be. It includes a bunch of feature we don't or can't use here and translations for all the languages. If we make a lite version here, we could add a double pass feature (using a cache) that would disable it if there are too many coordinates on a page. — [[User:Dispenser|Dispenser]] 19:17, 15 January 2010 (UTC)

{{hidden begin|title=WMA loader lite}}

<source lang="javascript">

// WikiMiniAtlas loader lite

var wikiminiatlas = {

config:

{

width : 600,

height : 400,

zoom : -1,

enabled : true,

onlytitle : false,

iframeurl : 'http://stable.toolserver.org/wma/iframe.html',

imgbase : 'http://stable.toolserver.org/wma/tiles/'

},

link : null,

bodyc : null,

coordinates : 0,

iframe : { div: null, iframe: null, closebutton: null },

mapbutton: null,

marker : { lat:0, lon:0 },

coord_params: '',

coord_filter: /^([\d+-.]+)_([\d+-.]*)_?([\d+-.]*)_?([NS])_([\d+-.]+)_([\d+-.]*)_?([\d+-.]*)_?([EOW])/,

// vertikale position auf der Seite bestimmen

totalOffset : function( obj, offset )

{

if( obj.offsetParent == null ||

obj.offsetParent.id == 'content' )

return offset + obj.offsetTop;

else

return wikiminiatlas.totalOffset(obj.offsetParent, offset+obj.offsetTop );

},

// move iframe around and toggle visibility

toggleIFrame : function( mp, my ) {

var newurl = wikiminiatlas.config.iframeurl + '?' + mp;

with(wikiminiatlas.iframe) {

if( iframe.src != newurl ) {

iframe.src = newurl;

div.style.top = (my || 0) + 'px';

div.style.visibility="visible";

} else {

div.style.visibility=(div.style.visibility == "visible"?"hidden":"visible");

}

return false;

},

// Insert the IFrame into the page.

loader : function() {

// apply user settings

if( typeof(wma_settings) == 'object' )

for (var key in wma_settings) {

if( typeof(wma_settings[key]) == typeof(wikiminiatlas.config[key]) )

wikiminiatlas.config[key] = wma_settings[key];

}

with(wikiminiatlas) {

bodyc = ( config.onlytitle ? document.getElementById('coordinates') : document.getElementById('bodyContent') || document);

if( config.enabled == false ) return;

if( bodyc == null ) return;

var links = bodyc.getElementsByTagName('a');

for( var key=0; (link = links[key])!=null; key++ ) {

if(link.className!='external text'

|| link.href.indexOf('http://stable.toolserver.org/geohack/geohack.php?')==-1

|| link.href.test(/_globe:(?!earth)/i)) {

continue;

}

coord_params = link.href.match(/&params=([^&=<>|]{7,255})/);

if(!coord_params) {

continue;

}

coord_params = coord_params[1];

if(coord_filter.test(coord_params)) {

coord_filter.exec(coord_params);

marker.lat=(1.0*RegExp.$1) + ((RegExp.$2||0)/60.0) + ((RegExp.$3||0)/3600.0);

if(RegExp.$4=='S') marker.lat*=-1;

marker.lon=(1.0*RegExp.$5) + ((RegExp.$6||0)/60.0) + ((RegExp.$7||0)/3600.0);

if(RegExp.$8=='W') marker.lon*=-1;

}

var zoomlevel = 1;

// Find a sensible Zoom-level based on type

if( coord_params.indexOf('_type:landmark') >= 0 ) zoomlevel = 8;

else if( coord_params.indexOf('_type:city') >= 0 ) zoomlevel = 4;

var ds_filter = /(dim|scale):([\d+-.]+)/;

// If given use dim or scale for a zoomlevel

if( ds_filter.test( clc ) )

{

ds_filter.exec(coord_params);

// wma shows dim approx 4e7m at zoom 0 or 1.5e8 is the scale of zoomlevel 0

zoomlevel = (RegExp.$1 == 'dim' ? Math.log( 4e7/RegExp.$2 ) : Math.log( 1.5e8/RegExp.$2 ) ) / Math.log( 2 );

if( zoomlevel > 10 ) zoomlevel = 10;

}

if( config.zoom != -1 ) {

var zoomlevel = config.zoom;

}

mapbutton = link.parentNode.insertBefore(document.createElement('img'), link);

mapbutton.alt = ' ';

mapbutton.className = 'noprint';

mapbutton.title = 'show ___location on an interactive map';

mapbutton.src = 'http://upload.wikimedia.org/wikipedia/commons/thumb/9/9a/Erioll_world.svg/18px-Erioll_world.svg.png'; //[[File:Erioll_world.svg]]

mapbutton.style.cssText = "padding:0px 3px 0px 0px; cursor:pointer;";

mapbutton.mapparam = (new Arrary(marker.lat, marker.lon, config.width, config.height, 'en', zoomlevel, wgUserLanguage)).join('_')

mapbutton.onclick = function() {

wikiminiatlas.toggleIFrame( this.mapparam, wikiminiatlas.totalOffset( this, 0 ) + 20; );

return true;

};

} //for

// prepare iframe to house the map

if ( coord_params != null ) { // FIXME: don't assume param= exists

iframe.div = document.createElement('div');

iframe.div.style.width = (config.width+2)+'px';

iframe.div.style.height = (config.height+2)+'px';

iframe.div.style.cssText = 'visibility:hidden; margin:0px; padding:0px; backgroundColor:white; position:absolute; right:2em; top:1em; border:1px solid gray; z-index:13;';

iframe.iframe = document.createElement('iframe');

iframe.iframe.scrolling = 'no';

iframe.iframe.frameBorder = '0';

iframe.iframe.style.width = (config.width)+'px';

iframe.iframe.style.height = (config.height)+'px';

iframe.iframe.style.cssText = 'position:absolute; right:1px; top:1px; margin:0px; padding:0px; z-index:14;';

iframe.div.appendChild(iframe.iframe);

iframe.closebutton = iframe.div.appendChild(document.createElement('img'));

iframe.closebutton.alt = '';

iframe.closebutton.src = 'http://upload.wikimedia.org/wikipedia/commons/d/d4/Button_hide.png' // [[File:Button hide.png]]

iframe.closebutton.style.cssText = 'z-Index:15; position:absolute; right:11px; top:9px; width:18px; cursor:pointer';

iframe.closebutton.title = 'close';

iframe.closebutton.mapparam = '';

iframe.closebutton.onclick = toggleIFrame;

var content = document.getElementById('content') || document.getElementById('mw_content') || document.body;

content.insertBefore(iframe.div,content.childNodes[0]);

}

} //with

} //function

}

addOnloadHook(wikiminiatlas.loader);

//</source>

{{hidden end}}

I was hoping to get the size a lot smaller, but I guess this will have to do. It is currently at approx. 150 lines/6 KB compared to the original 503 lines/16 KB.

*Eliminate languages (~100 lines/4 KB)

*Removed unimplemented functions

*Use the cssText property (all major browser support this)

*Figure out a way of using regexes to reduce the D/DM/DMS parser to ten lines

*Look at rewriting parts to use less code

This code is completely untested, hopefully it'll serve as a spring board for others. — [[User:Dispenser|Dispenser]] 21:33, 15 January 2010 (UTC)

:Ok, let me say one thing first: forking is a bad idea. That being said, good idea with the new D/DM/DMS parsing. This should be moved to the WMA production code on meta. The res I'm not excited about. Sure, going from 503 to 150 lines sounds impressive, but I'd say it is about 100% irrelevant. The tokenization of a few hundred Javascript lines should be no factor in the overall execution speed in pretty much any browser. The fact that the code still has to iterate over all links remains unchanged. Having a central code repository and the easy maintainability that comes with it should have an infinitely higher priority. We used to have local copies of WMA on every wiki. It sucked. It was a maintenance hell. We dropped that for good reasons. Please do not bring it back. --[[User:Dschwen|Dschwen]] 23:44, 15 January 2010 (UTC)

::My goal was 50 lines so I fell quite far from that. But if I rewrite it I'm pretty sure I can get it under 100. I purposely did not add in the caching feature since I did not want to change the feature set/current limitations. Now performance, from my experience with JavaScript, the slowest part is the attachment of the icons next to the links since most browser choose to re-render the page at this point. I haven't found a way to avoid this. The only solution that I have is to time it, and project the total time, if too high then abort. Finally, on maintenance, the thread is about incorporation at least part of the WMA loader script, my goal is to make it lean enough that we avoid the extra overhead of contacting another server. — [[User:Dispenser|Dispenser]] 03:28, 16 January 2010 (UTC)

:::Well, that addresses none of my points. Linenumbers is a completely irrelevant performance metric that may impress people who know pretty much nothing about programming. Forking the loader and have instances on every wiki is a maintenance nightmare. --[[User:Dschwen|Dschwen]] 14:58, 16 January 2010 (UTC)

::::I was looking at isMaplink() i'm guessing it's very inefficient for it's purpose. I think it's better to construct one regexp for the urls early on, and then match the title to that regexp, instead of doing many matches inside a for loop. That might bring a significant speedup. —[[User:TheDJ|Th<span style="color: green">e</span>DJ]] ([[User talk:TheDJ|talk]] • [[Special:Contributions/TheDJ|contribs]]) 22:15, 18 January 2010 (UTC)

:::::I added a five second timeout to the script (clear cache to test). [[List of bridges on the National Register of Historic Places in Pennsylvania]] is a nice test case. In Google Chrome all coordinates are processed in a snap. Quite a shame that the other major browsers are left in the dust like this. --[[User:Dschwen|Dschwen]] 15:38, 19 January 2010 (UTC)

:::::I ported Dispensers regexp over to the production code, removing the call to ismaplink. The speedup is, to my surprise, pretty much insignificant. The amount of coordinates that get processed within the 5sec timelimit increases a bit, but it is still only about a quarter of all coords in Firefox, while Google Chrome manages to process 100% of the coordinates on that page, even with the old code. --[[User:Dschwen|Dschwen]] 16:26, 19 January 2010 (UTC)

::::::Another improvement might be to set the properties of the imagemap before inserting it into the visible DOM. That might avoid a few DOM updates. —[[User:TheDJ|Th<span style="color: green">e</span>DJ]] ([[User talk:TheDJ|talk]] • [[Special:Contributions/TheDJ|contribs]]) 17:02, 19 January 2010 (UTC)

:::::::And addHandler(imagemap, "click", hookFunct); should be used instead of onclick, because .onclick behaves unpredictably when multiple onclick are hooked. —[[User:TheDJ|Th<span style="color: green">e</span>DJ]] ([[User talk:TheDJ|talk]] • [[Special:Contributions/TheDJ|contribs]]) 17:06, 19 January 2010 (UTC)

:::::::: Which onclick are you referring to. In principle I agree about addHandler, but in the WMA, I ''know'' that by design only one click handler exists for each button. No need to use the slower addHandler. <s>Will try your other suggestion about the DOM updates.</s> --[[User:Dschwen|Dschwen]] 17:54, 19 January 2010 (UTC)

::::::::: Ok, not feasable. The hookUpMapbutton function does not perform visible DOM changes, so there is no reason for a slowdown (other than the browser developers being just plain dumb). Furthermore hookUpMapbutton accesses the position of the element on the page, and to obtain that the element needs to be hooked into the document. --[[User:Dschwen|Dschwen]] 17:54, 19 January 2010 (UTC)

::::::::::I realize now that I was looking at the old version apparently. Current is much better. —[[User:TheDJ|Th<span style="color: green">e</span>DJ]] ([[User talk:TheDJ|talk]] • [[Special:Contributions/TheDJ|contribs]]) 19:27, 19 January 2010 (UTC)