r53347 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r53346‎ | r53347 | r53348 >
Date:02:58, 16 July 2009
Author:simetrical
Status:ok (Comments)
Tags:
Comment:
Use capability testing instead of UA sniffing

This should theoretically work fine, but I'm admittedly not *totally*
sure what this is supposed to do, so I haven't actually tested it. The
old behavior was almost certainly wrong, though: is_khtml was returning
true for all WebKit, and non-ancient WebKit definitely supports
overflow-x. Robert Stojnic/rainman should take a look at this to make
sure it's right, or anyone else who knows what this does.
Modified paths:
  • /trunk/phase3/skins/common/mwsuggest.js (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/common/mwsuggest.js
——@@ -107,9 +107,9 @@
108108 }
109109
110110 function os_operaWidthFix(x){
111 - // TODO: better css2 incompatibility detection here
112 - if(is_opera || is_khtml || navigator.userAgent.toLowerCase().indexOf('firefox/1')!=-1){
113 - return 30; // opera&konqueror & old firefox don't understand overflow-x, estimate scrollbar width
 111+ // For browsers that don't understand overflow-x, estimate scrollbar width
 112+ if(typeof document.body.style.overflowX != "string"){
 113+ return 30;
114114 }
115115 return 0;
116116 }

Comments

#Comment by Simetrical (talk | contribs)   15:36, 17 August 2009

I discussed this with Robert, and he said the issue is when you have long page names. I tested with a long page name and verified that this patch improves appearance significantly on current WebKit and Opera: it cuts the box to the correct length instead of leaving a bunch of ugly whitespace. It also looks good on current Konqueror. I didn't test on the old browser versions the fix was originally written for, since I don't have any of them handy, but it should work in theory!

Status & tagging log