Last modified: 2013-01-07 20:12:22 UTC
Currently, when you activate the "edit on double click" setting, any double-click anywhere on the body will open the edit form. I have found that to be somewhat confusing already, for example when mistakenly double-clicking on the sidebar, the search box or other parts outside the actual page content. Thus, I believe that this script should probably be added via Resource Loader as an event listener on div#bodyContent instead of on the <body> tag, as it currently is the case.
It was added directly to the body tag. $bodyAttrs['ondblclick'] = "document.location = '" . Xml::escapeJsString( $this->getTitle()->getEditURL() ) . "'"; However, <div id="bodyContent"> is added dirrectly by the skin, it doesn't seem a good idea to add it there. So yes, an event listener seems the way to go.
The id and locatin of that tag is variable per-skin. The mw.util class has a $content variable though which contains a jQuery object of the content element in the current skin. should be as simple as attaching a .dblclick() to mw.util.$content
Created attachment 8531 [details] proposed patch for this bug Krinkle, I used the skin-independent mw.util.$content variable like you suggested. I removed the php-generated 'ondblclick' attribute to <body> and made a jQuery event listener. This solution works for me.
(In reply to comment #3) > Created attachment 8531 [details] > proposed patch for this bug > > Krinkle, I used the skin-independent mw.util.$content variable like you > suggested. I removed the php-generated 'ondblclick' attribute to <body> and > made a jQuery event listener. This solution works for me. Hi Nik, Thanks for your patch. Two notes though: 1) document.location was originally a read-only property, although Gecko browsers (like Firefox) allow you to assign to it as well. For cross-browser safety, use window.location instead. [1] 2) Appending '&action=url' to the url is not safe and prone to error. The following three urls are all examples of how MediaWiki can be configured that are popular and supported: - Using rewrite rules with wgArticlePath: -- http://somewi.ki/wiki/Article - Using rewrite rules and wgActionPaths: -- http://somewi.ki/view/Article - The default: -- http://somewi.ki/w/index.php/Article -- Full url (almost never used, but works) - http://somewi.ki/w/index.php?title=Article Appending '&action' only works for the last one. I suggest using the wgScript mw.config variable ( mw.config.get( 'wgScript' ) ), and manually building a url with the title (wgPageName) and action (edit) parameters. -- Krinkle [1] https://developer.mozilla.org/en/document.location#Notes
Created attachment 8547 [details] patch revision Krinkle, thanks for your suggestions. I've revised the patch, the edit URL is now reliable since I'm building it manually instead of appending to the existing URL. However, this means that when double-clicking to edit, the edit URL will always be in this format: - http://somewi.ki/wiki/index.php?title=Main_Page&action=edit Regardless of the URL format of the rest of the wiki.
Nice to see the follow-ups on this bug. I've recently noted another problem with this event listener that would be nice to fix already in this revision. We shouldn't use an anonymous function here, as these can't be removed via Element.removeEventListener(). Removing them would be useful for scripts like Twinkle: Currently, double-clicking a page while using Twinkle opens the edit mode, which is quite annoying.
(In reply to comment #6) > Nice to see the follow-ups on this bug. I've recently noted another problem > with this event listener that would be nice to fix already in this revision. We > shouldn't use an anonymous function here, as these can't be removed via > Element.removeEventListener(). Removing them would be useful for scripts like > Twinkle: Currently, double-clicking a page while using Twinkle opens the edit > mode, which is quite annoying. Remember that this is an opt-in preference. Either Twinkle users shouldn't enable that preference or Twink should $content.unbind('dblclick') or something like that (perhaps namespace the event)
@Nik: I've reviewed your patch. It looks good. If you're familiar with event namespaces (in particular in jQuery) it would be nice if you could bind it to "dblclick" with the "mw-editondblclick" namespace so that scritps can use .unbind() for that namespace only without removing all events. It's a bit beyond the scope of "easy" though, so if you don't feel like doing that I will apply your good patch as-is and then apply an additional fix for the namespace. Thanks in advance,
Patch has several bugs: * fails to take revision ID (oldid parameter) into account, and possibly others * hardcodes use of script & action=edit instead of using $wgActionPaths * clutters mediawiki.util.js with rarely-used code for all users I would recommend: * grab the edit URL from the edit tab to avoid having to recalculate it manually (which causes the first two bugs above) * move from mediawiki.util.js to its own module, and only add that module when the option is being used
(patch also doesn't do URL escaping on the title, which can cause failures when eg title contains a literal &)
nice catc(In reply to comment #9) > Patch has several bugs: Nice catches. A few more: * Loading of module needs to be moved out of OutputPage->headElement probably * Should account for the situation in which there is no edit link on the page (ie. not bind the event, because if MediaWiki decided not to output that tab, it probably has a good reason) * There ar differences in skins in where this link will be. This is a common problem we are encountering more and more often lately (just look at the code that makes mw.util.$content and addPortletLink work, it's disguisting). Probably need to make some radical changes to skins in the future, but oh well. This can be fixed in the long term by not allowing skins to use different IDs for core elements or by requiring skins to log into a method how/where elements are) – back to the present this can be solved by either: -- exporting the edit-tab link through a config variable (alert! this is not a config variable) -- exporting it as a piece of information in the per-module configuration (sounds good, except that such a thing doesn't exist yet) -- scrapping HTML for "#ca-edit a" and not caring about other skins. -- a switch-case for all skins. The last two options will be ugly and not extendable by custom skins.
Created attachment 8759 [details] Follow-up patch implementing the points above
Sorry for the long delay. Patch applied in r93664.
