Back in May 2004, Gabriel Wicke was creating a neat new skin called Monobook. Unlike the old skins, it used good semantic markup with CSS 2 for style. Gabriel made sure to test in a lot of browsers and made up files full of extensive fixes for browsers that had problems.
One such browser was the default KDE browser, Konqueror. Even relatively web-savvy people have barely heard of it and never used it. He was nice and checked whether it worked properly in his new skin anyway. Since it didn't, he committed a quick fix in r3532 to eliminate some horizontal scrollbars. Then everyone forgot about it, because nobody uses KHTML.
It turns out there was a slight problem with his fix. He loaded it based on this code:
var is_khtml = (navigator.vendor == 'KDE' || ( document.childNodes && !document.all && !navigator.taintEnabled ));
The problem here is pretty straightforward. A bug fix is being loaded, without checking to see whether the bug exists. The fix is loaded for all versions of KHTML past, present, and *future*. If the KHTML devs fixed the bug, then they'd have a bogus stylesheet being loaded that would mess up their display, and they couldn't do anything about it.
Well, nobody much used or uses KHTML. But it just so happens that in 2003, Apple debuted a new web browser based on a fork of KHTML. And in 2008, Google debuted another browser based on the same rendering engine. And if you add them together, they now have 6% market share or more. And we've still been serving them this broken KHTML fixes file for something that was fixed eons ago.
Just recently, in WebKit r47255, they changed their code to better match other browsers' handling of "almost standards mode". They removed some quirk that was allowing them to render correctly despite the bogus CSS we were serving them. And so suddenly they're faced with the prospect of having to use a site-specific hack ("if path ends in /KHTMLFixes.css, ignore the file") because we screwed up. See their bug here: https://bugs.webkit.org/show_bug.cgi?id=28350
I had already killed KHTMLFixes.css in r53141, but it's still in every MediaWiki release since 1.5. And this isn't the only time this has happened. A while back someone committed some fixes for Opera RTL. They loaded the fixes for, yes, Opera version 9 or greater, or some similar check. When I checked on Opera 9.6, I found that the fix was degrading display, not improving it.
Sometimes we need to do browser sniffing of some kind, because sometimes browsers don't implement standards properly. There are two ways to do it that are okay:
1) Capability testing. If possible, just check directly whether the browser can do it. This works best with JS functionality, for instance in getElementsByClassName in wikibits.js:
if ( typeof( oElm.getElementsByClassName ) == "function" ) { /* Use a native implementation where possible FF3, Saf3.2, Opera 9.5 */
It can also be used in other cases sometimes. For instance, in r53347 I made this change:
- // TODO: better css2 incompatibility detection here - if(is_opera || is_khtml || navigator.userAgent.toLowerCase().indexOf('firefox/1')!=-1){ - return 30; // opera&konqueror & old firefox don't understand overflow-x, estimate scrollbar width + // For browsers that don't understand overflow-x, estimate scrollbar width + if(typeof document.body.style.overflowX != "string"){ + return 30;
Instead of using a hardcoded list of browsers that didn't support overflow-x, I checked whether the overflowX property existed. This isn't totally foolproof, but it sure bets assuming that no future version of Opera or KHTML will support overflow-x. (I'm pretty sure both already do, in fact.)
2) "Version <= X." If it's not reasonable to check capabilities, then at least allow browser implementers to fix their bugs in future versions. If you find that all current versions of Firefox do something or other incorrectly, then don't serve incorrect content to all versions of Firefox. In that case, during Firefox 3.6 development, they'll find out that their improvements to standards compliance cause Wikipedia to break! Instead, serve incorrect content to Firefox 3.5 or less, and standard markup to all greater versions. That way, during 3.6 development, they'll find out that their *failure* to comply with standards causes Wikipedia to break. With any luck, that will encourage them to fix the problem instead of punishing them. It's not as good as being able to automatically serve the right content if they haven't fixed things, but it's better than serving bad content forever.
I tried to remove some browser-sniffing from wikibits.js, but there's undoubtedly some I missed. Especially with the large amounts of JS being added recently for usability/new upload/etc., could everyone *please* check to make sure that there are no broken browser checks being committed? This kind of thing hurts our users in the long term (especially third parties who don't upgrade so often), and is really unfair to browser developers who are trying to improve their standards compliance. Thanks!
In theory at least all new checks in the last few years have been specific in this way -- either testing for capabilities or if that's not doable looking for particular versions (like the IE fixes which are tied to known releases of IE, or the Gecko and WebKit fixes which check if they're on a version of the engine prior to the bug fix.)
Definitely keep an eye out and make sure we don't regress on that, everybody! :)
-- brion vibber (brion @ wikimedia.org)
On Aug 17, 2009, at 7:41, Aryeh Gregor Simetrical+wikilist@gmail.com wrote:
Back in May 2004, Gabriel Wicke was creating a neat new skin called Monobook. Unlike the old skins, it used good semantic markup with CSS 2 for style. Gabriel made sure to test in a lot of browsers and made up files full of extensive fixes for browsers that had problems.
One such browser was the default KDE browser, Konqueror. Even relatively web-savvy people have barely heard of it and never used it. He was nice and checked whether it worked properly in his new skin anyway. Since it didn't, he committed a quick fix in r3532 to eliminate some horizontal scrollbars. Then everyone forgot about it, because nobody uses KHTML.
It turns out there was a slight problem with his fix. He loaded it based on this code:
var is_khtml = (navigator.vendor == 'KDE' || ( document.childNodes && !document.all && !navigator.taintEnabled ));
The problem here is pretty straightforward. A bug fix is being loaded, without checking to see whether the bug exists. The fix is loaded for all versions of KHTML past, present, and *future*. If the KHTML devs fixed the bug, then they'd have a bogus stylesheet being loaded that would mess up their display, and they couldn't do anything about it.
Well, nobody much used or uses KHTML. But it just so happens that in 2003, Apple debuted a new web browser based on a fork of KHTML. And in 2008, Google debuted another browser based on the same rendering engine. And if you add them together, they now have 6% market share or more. And we've still been serving them this broken KHTML fixes file for something that was fixed eons ago.
Just recently, in WebKit r47255, they changed their code to better match other browsers' handling of "almost standards mode". They removed some quirk that was allowing them to render correctly despite the bogus CSS we were serving them. And so suddenly they're faced with the prospect of having to use a site-specific hack ("if path ends in /KHTMLFixes.css, ignore the file") because we screwed up. See their bug here: https://bugs.webkit.org/show_bug.cgi?id=28350
I had already killed KHTMLFixes.css in r53141, but it's still in every MediaWiki release since 1.5. And this isn't the only time this has happened. A while back someone committed some fixes for Opera RTL. They loaded the fixes for, yes, Opera version 9 or greater, or some similar check. When I checked on Opera 9.6, I found that the fix was degrading display, not improving it.
Sometimes we need to do browser sniffing of some kind, because sometimes browsers don't implement standards properly. There are two ways to do it that are okay:
- Capability testing. If possible, just check directly whether the
browser can do it. This works best with JS functionality, for instance in getElementsByClassName in wikibits.js:
if ( typeof( oElm.getElementsByClassName ) == "function" ) { /* Use a native implementation where possible FF3, Saf3.2, Opera 9.5 */
It can also be used in other cases sometimes. For instance, in r53347 I made this change:
- // TODO: better css2 incompatibility detection here
- if(is_opera || is_khtml ||
navigator.userAgent.toLowerCase().indexOf('firefox/1')!=-1){
return 30; // opera&konqueror & old firefox don't understand
overflow-x, estimate scrollbar width
- // For browsers that don't understand overflow-x, estimate
scrollbar width
- if(typeof document.body.style.overflowX != "string"){
return 30;
Instead of using a hardcoded list of browsers that didn't support overflow-x, I checked whether the overflowX property existed. This isn't totally foolproof, but it sure bets assuming that no future version of Opera or KHTML will support overflow-x. (I'm pretty sure both already do, in fact.)
- "Version <= X." If it's not reasonable to check capabilities, then
at least allow browser implementers to fix their bugs in future versions. If you find that all current versions of Firefox do something or other incorrectly, then don't serve incorrect content to all versions of Firefox. In that case, during Firefox 3.6 development, they'll find out that their improvements to standards compliance cause Wikipedia to break! Instead, serve incorrect content to Firefox 3.5 or less, and standard markup to all greater versions. That way, during 3.6 development, they'll find out that their *failure* to comply with standards causes Wikipedia to break. With any luck, that will encourage them to fix the problem instead of punishing them. It's not as good as being able to automatically serve the right content if they haven't fixed things, but it's better than serving bad content forever.
I tried to remove some browser-sniffing from wikibits.js, but there's undoubtedly some I missed. Especially with the large amounts of JS being added recently for usability/new upload/etc., could everyone *please* check to make sure that there are no broken browser checks being committed? This kind of thing hurts our users in the long term (especially third parties who don't upgrade so often), and is really unfair to browser developers who are trying to improve their standards compliance. Thanks!
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Mon, Aug 17, 2009 at 12:30 PM, Roan Kattouwroan.kattouw@gmail.com wrote:
Both the usabilty and new upload JS use jQuery, which hides most browser sniffing you need in abstraction layers. To my knowledge the jQuery devs do sniff browsers properly, and if something should go wrong in jQuery, it's not just Wikipedia that'll break.
This doesn't look like doing things properly to me:
js/js2/jquery-ui-1.7.2.js- // Prevent text selection in IE js/js2/jquery-ui-1.7.2.js: if ($.browser.msie) { js/js2/jquery-ui-1.7.2.js- this._mouseUnselectable = this.element.attr('unselectable'); js/js2/jquery-ui-1.7.2.js- this.element.attr('unselectable', 'on'); js/js2/jquery-ui-1.7.2.js- }
js/js2/jquery-ui-1.7.2.js- // preventDefault() is used to prevent the selection of text here - js/js2/jquery-ui-1.7.2.js- // however, in Safari, this causes select boxes not to be selectable js/js2/jquery-ui-1.7.2.js- // anymore, so this fix is needed js/js2/jquery-ui-1.7.2.js: ($.browser.safari || event.preventDefault());
js/js2/jquery-ui-1.7.2.js- return { js/js2/jquery-ui-1.7.2.js- top: ( js/js2/jquery-ui-1.7.2.js- pos.top // The absolute mouse position js/js2/jquery-ui-1.7.2.js- + this.offset.relative.top * mod // Only for relative positioned nodes: Relative offset from element to offset parent js/js2/jquery-ui-1.7.2.js- + this.offset.parent.top * mod // The offsetParent's offset without borders (offset + border) js/js2/jquery-ui-1.7.2.js: - ($.browser.safari && this.cssPosition == 'fixed' ? 0 : ( this.cssPosition == 'fixed' ? -this.scrollParent.scrollTop() : ( scrollIsRootNode ? 0 : scroll.scrollTop() ) ) * mod) js/js2/jquery-ui-1.7.2.js- ), js/js2/jquery-ui-1.7.2.js- left: ( js/js2/jquery-ui-1.7.2.js- pos.left // The absolute mouse position js/js2/jquery-ui-1.7.2.js- + this.offset.relative.left * mod // Only for relative positioned nodes: Relative offset from element to offset parent js/js2/jquery-ui-1.7.2.js- + this.offset.parent.left * mod // The offsetParent's offset without borders (offset + border) js/js2/jquery-ui-1.7.2.js: - ($.browser.safari && this.cssPosition == 'fixed' ? 0 : ( this.cssPosition == 'fixed' ? -this.scrollParent.scrollLeft() : scrollIsRootNode ? 0 : scroll.scrollLeft() ) * mod) js/js2/jquery-ui-1.7.2.js- ) js/js2/jquery-ui-1.7.2.js- }; js/js2/jquery-ui-1.7.2.js- js/js2/jquery-ui-1.7.2.js- },
js/js2/jquery-ui-1.7.2.js- //Opera fix for relative positioning js/js2/jquery-ui-1.7.2.js: if (/relative/.test(this.element.css('position')) && $.browser.opera) js/js2/jquery-ui-1.7.2.js- this.element.css({ position: 'relative', top: 'auto', left: 'auto' });
js/js2/jquery-ui-1.7.2.js- // Prevent IE from keeping other link focussed when using the back button js/js2/jquery-ui-1.7.2.js- // and remove dotted border from clicked link. This is controlled via CSS js/js2/jquery-ui-1.7.2.js- // in modern browsers; blur() removes focus from address bar in Firefox js/js2/jquery-ui-1.7.2.js- // which can become a usability and annoying problem with tabs('rotate'). js/js2/jquery-ui-1.7.2.js: if ($.browser.msie) { js/js2/jquery-ui-1.7.2.js- this.blur(); js/js2/jquery-ui-1.7.2.js- }
etc. There are lots more like that. And that's only one file, and only the ones that looked like they were clearly wrong. It seems to be part of jQuery itself and not our code, but there are examples from our code too. Like r55269, which I've already commented on. (Although it apparently was already scapped less than two hours after it was committed, before I or probably almost anyone else had a chance to look at it, when we have thousands of commits dating back more than two months that are still not live?)
We have some things in core as well, like this in wikibits.js:
if (is_gecko) { // Mozilla needs to wait until after load, otherwise the window doesn't scroll addOnloadHook(function () { if (window.location.hash == "") window.location.hash = fragment; }); } else {
Is this necessary for recent FF versions? I don't know. From looking at it, it will degrade the user experience, causing a jump possibly even after the user has already started scrolling.
2009/8/17 Aryeh Gregor Simetrical+wikilist@gmail.com:
I tried to remove some browser-sniffing from wikibits.js, but there's undoubtedly some I missed. Especially with the large amounts of JS being added recently for usability/new upload/etc., could everyone *please* check to make sure that there are no broken browser checks being committed?
Both the usabilty and new upload JS use jQuery, which hides most browser sniffing you need in abstraction layers. To my knowledge the jQuery devs do sniff browsers properly, and if something should go wrong in jQuery, it's not just Wikipedia that'll break.
Roan Kattouw (Catrope)
On Aug 17, 2009, at 9:30, Roan Kattouw roan.kattouw@gmail.com wrote:
Both the usabilty and new upload JS use jQuery, which hides most browser sniffing you need in abstraction layers. To my knowledge the jQuery devs do sniff browsers properly, and if something should go wrong in jQuery, it's not just Wikipedia that'll break.
Yeah, the current jquery release claims to be entirely using capability checks rather than agent sniffing, which makes me warm and fuzzy. :)
-- brion vibber (brion @ wikimedia.org)
wikitech-l@lists.wikimedia.org