r77576 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r77575‎ | r77576 | r77577 >
Date:12:56, 2 December 2010
Author:jojo
Status:resolved (Comments)
Tags:
Comment:
make things work w/ upcoming MW 1.17 again
Modified paths:
  • /trunk/extensions/Collection/Collection.body.php (modified) (history)
  • /trunk/extensions/Collection/Collection.hooks.php (modified) (history)
  • /trunk/extensions/Collection/js/bookcreator.js (modified) (history)

Diff [purge]

Index: trunk/extensions/Collection/Collection.hooks.php
——@@ -252,20 +252,14 @@
253253 }
254254 }
255255
256 - $html = '';
 256+ $html = Xml::element( 'script',
 257+ array(
 258+ 'type' => $wgJsMimeType,
 259+ 'src' => "$jsPath/jquery.js?$wgCollectionStyleVersion",
 260+ ),
 261+ '', false
 262+ );
257263
258 - if ( method_exists( $wgOut, 'includeJQuery' ) ) {
259 - $wgOut->includeJQuery();
260 - } else {
261 - $html .= Xml::element( 'script',
262 - array(
263 - 'type' => $wgJsMimeType,
264 - 'src' => "$jsPath/jquery.js?$wgCollectionStyleVersion",
265 - ),
266 - '', false
267 - );
268 - }
269 -
270264 $html .= Xml::element( 'script',
271265 array(
272266 'type' => $wgJsMimeType,
Index: trunk/extensions/Collection/js/bookcreator.js
——@@ -21,141 +21,142 @@
2222
2323 (function($) {
2424
25 -var script_url = wgServer + ((wgScript == null) ? (wgScriptPath + "/index.php") : wgScript);
 25+$(function() {
2626
27 -function save_collection(collection) {
28 - $.jStorage.set('collection', collection);
29 -}
 27+ var script_url = wgServer + ((wgScript == null) ? (wgScriptPath + "/index.php") : wgScript);
3028
31 -window.wfCollectionSave = save_collection;
 29+ function save_collection(collection) {
 30+ $.jStorage.set('collection', collection);
 31+ }
3232
33 -function refreshBookCreatorBox(hint, oldid) {
34 - $.getJSON(script_url, {
35 - 'action': 'ajax',
36 - 'rs': 'wfAjaxCollectionGetBookCreatorBoxContent',
37 - 'rsargs[]': [hint, oldid]
38 - }, function(result) {
39 - $('#coll-book_creator_box').html(result.html);
40 - });
41 -}
 33+ window.wfCollectionSave = save_collection;
4234
43 -function collectionCall(func, args) {
44 - var hint = args.shift();
45 - $.post(script_url, {
46 - 'action': 'ajax',
47 - 'rs': 'wfAjaxCollection' + func,
48 - 'rsargs[]': args
49 - }, function(result) {
50 - var oldid = null;
51 - if (args.length == 3) {
52 - oldid = args[2];
53 - }
54 - refreshBookCreatorBox(hint, oldid);
55 - save_collection(result.collection);
56 - }, 'json');
57 -}
 35+ function refreshBookCreatorBox(hint, oldid) {
 36+ $.getJSON(script_url, {
 37+ 'action': 'ajax',
 38+ 'rs': 'wfAjaxCollectionGetBookCreatorBoxContent',
 39+ 'rsargs[]': [hint, oldid]
 40+ }, function(result) {
 41+ $('#coll-book_creator_box').html(result.html);
 42+ });
 43+ }
5844
59 -window.collectionCall = collectionCall; // public
 45+ function collectionCall(func, args) {
 46+ var hint = args.shift();
 47+ $.post(script_url, {
 48+ 'action': 'ajax',
 49+ 'rs': 'wfAjaxCollection' + func,
 50+ 'rsargs[]': args
 51+ }, function(result) {
 52+ var oldid = null;
 53+ if (args.length == 3) {
 54+ oldid = args[2];
 55+ }
 56+ refreshBookCreatorBox(hint, oldid);
 57+ save_collection(result.collection);
 58+ }, 'json');
 59+ }
6060
 61+ window.collectionCall = collectionCall; // public
6162
62 -var mouse_pos = {};
63 -var popup_div = null;
64 -var addremove_link = null;
65 -var visible = false;
66 -var show_soon_timeout = null;
67 -var get_data_xhr = null;
68 -var current_link = null;
69 -var title = null;
7063
71 -function createDiv() {
72 - addremove_link = $('<a href="javascript:void(0)" />');
73 - popup_div = $('<div id="collectionpopup" />');
74 - popup_div.append(addremove_link)
75 - $('body').append(popup_div);
76 - popup_div.hide();
77 -}
 64+ var mouse_pos = {};
 65+ var popup_div = null;
 66+ var addremove_link = null;
 67+ var visible = false;
 68+ var show_soon_timeout = null;
 69+ var get_data_xhr = null;
 70+ var current_link = null;
 71+ var title = null;
7872
79 -function addremove_article(action, title) {
80 - $.post(script_url, {
81 - 'action': 'ajax',
82 - 'rs': 'wfAjaxCollection' + action.charAt(0).toUpperCase() + action.slice(1) + 'Article',
83 - 'rsargs[]': [0, title, '']
84 - }, function(result) {
85 - hide();
86 - refreshBookCreatorBox(null, null);
87 - save_collection(result.collection);
88 - }, 'json');
89 -}
 73+ function createDiv() {
 74+ addremove_link = $('<a href="javascript:void(0)" />');
 75+ popup_div = $('<div id="collectionpopup" />');
 76+ popup_div.append(addremove_link)
 77+ $('body').append(popup_div);
 78+ popup_div.hide();
 79+ }
9080
91 -function show(link) {
92 - if (visible) {
93 - return;
94 - }
95 - current_link = link;
96 - title = link.attr('title');
97 - link.attr('title', ''); // disable default browser tooltip
98 - show_soon_timeout = setTimeout(function() {
99 - get_data_xhr = $.post(script_url, {
 81+ function addremove_article(action, title) {
 82+ $.post(script_url, {
10083 'action': 'ajax',
101 - 'rs': 'wfAjaxCollectionGetPopupData',
102 - 'rsargs[]': [title]
 84+ 'rs': 'wfAjaxCollection' + action.charAt(0).toUpperCase() + action.slice(1) + 'Article',
 85+ 'rsargs[]': [0, title, '']
10386 }, function(result) {
104 - visible = true;
105 - var img = $('<img />').attr({src: result.img, alt: ''});
106 - addremove_link
107 - .text('\u00a0' + result.text)
108 - .prepend(img)
109 - .unbind('click')
110 - .click(function(e) { addremove_article(result.action, result.title); });
111 - popup_div
112 - .css({left: mouse_pos.x + 2 + 'px',
113 - top: mouse_pos.y + 2 + 'px'})
114 - .show();
 87+ hide();
 88+ refreshBookCreatorBox(null, null);
 89+ save_collection(result.collection);
11590 }, 'json');
116 - }, 300);
117 -}
 91+ }
11892
119 -function cancel() {
120 - if (current_link && title) {
121 - current_link.attr('title', title);
 93+ function show(link) {
 94+ if (visible) {
 95+ return;
 96+ }
 97+ current_link = link;
 98+ title = link.attr('title');
 99+ link.attr('title', ''); // disable default browser tooltip
 100+ show_soon_timeout = setTimeout(function() {
 101+ get_data_xhr = $.post(script_url, {
 102+ 'action': 'ajax',
 103+ 'rs': 'wfAjaxCollectionGetPopupData',
 104+ 'rsargs[]': [title]
 105+ }, function(result) {
 106+ visible = true;
 107+ var img = $('<img />').attr({src: result.img, alt: ''});
 108+ addremove_link
 109+ .text('\u00a0' + result.text)
 110+ .prepend(img)
 111+ .unbind('click')
 112+ .click(function(e) { addremove_article(result.action, result.title); });
 113+ popup_div
 114+ .css({left: mouse_pos.x + 2 + 'px',
 115+ top: mouse_pos.y + 2 + 'px'})
 116+ .show();
 117+ }, 'json');
 118+ }, 300);
122119 }
123 - if (show_soon_timeout) {
124 - clearTimeout(show_soon_timeout);
125 - show_soon_timeout = null;
 120+
 121+ function cancel() {
 122+ if (current_link && title) {
 123+ current_link.attr('title', title);
 124+ }
 125+ if (show_soon_timeout) {
 126+ clearTimeout(show_soon_timeout);
 127+ show_soon_timeout = null;
 128+ }
 129+ if (get_data_xhr) {
 130+ get_data_xhr.abort();
 131+ get_data_xhr = null;
 132+ }
126133 }
127 - if (get_data_xhr) {
128 - get_data_xhr.abort();
129 - get_data_xhr = null;
 134+
 135+ function hide() {
 136+ cancel();
 137+ if (!visible) {
 138+ return;
 139+ }
 140+ visible = false;
 141+ popup_div.hide();
130142 }
131 -}
132143
133 -function hide() {
134 - cancel();
135 - if (!visible) {
136 - return;
 144+ function is_inside(x, y, left, top, width, height) {
 145+ var fuzz = 5;
 146+ return x + fuzz >= left && x - fuzz <= left + width
 147+ && y + fuzz >= top && y - fuzz <= top + height;
137148 }
138 - visible = false;
139 - popup_div.hide();
140 -}
141149
142 -function is_inside(x, y, left, top, width, height) {
143 - var fuzz = 5;
144 - return x + fuzz >= left && x - fuzz <= left + width
145 - && y + fuzz >= top && y - fuzz <= top + height;
146 -}
147 -
148 -function check_popup_hide() {
149 - if (!visible) {
150 - return;
 150+ function check_popup_hide() {
 151+ if (!visible) {
 152+ return;
 153+ }
 154+ var pos = popup_div.offset();
 155+ if (!is_inside(mouse_pos.x, mouse_pos.y,
 156+ pos.left, pos.top, popup_div.width(), popup_div.height())) {
 157+ hide();
 158+ }
151159 }
152 - var pos = popup_div.offset();
153 - if (!is_inside(mouse_pos.x, mouse_pos.y,
154 - pos.left, pos.top, popup_div.width(), popup_div.height())) {
155 - hide();
156 - }
157 -}
158160
159 -$(function() {
160161 $(document).mousemove(function(e) {
161162 mouse_pos.x = e.pageX;
162163 mouse_pos.y = e.pageY;
Index: trunk/extensions/Collection/Collection.body.php
——@@ -321,12 +321,8 @@
322322 $imagepath = "$wgScriptPath/extensions/Collection/images";
323323 $jspath = "$wgScriptPath/extensions/Collection/js";
324324
325 - if ( method_exists( $wgOut, 'includeJQuery' ) ) {
326 - $wgOut->includeJQuery();
327 - } else {
328 - $wgOut->addScript( "<script type=\"$wgJsMimeType\" src=\"$jspath/jquery.js?" .
329 - "$wgCollectionStyleVersion\"></script>" );
330 - }
 325+ $wgOut->addScript( "<script type=\"$wgJsMimeType\" src=\"$jspath/jquery.js?" .
 326+ "$wgCollectionStyleVersion\"></script>" );
331327 $wgOut->addScript( "<script type=\"$wgJsMimeType\" src=\"$jspath/jquery.json.js?" .
332328 "$wgCollectionStyleVersion\"></script>" );
333329 $wgOut->addScript( "<script type=\"$wgJsMimeType\" src=\"$jspath/jstorage.js?" .

Comments

#Comment by Catrope (talk | contribs)   14:08, 2 December 2010

You really, really should not include your own jQuery unconditionally. This is unnecessary and can cause bugs in a ResourceLoader environment. I'll poke at fixing this.

#Comment by Catrope (talk | contribs)   16:24, 9 January 2011

Resolved in r79865.

#Comment by Catrope (talk | contribs)   16:25, 9 January 2011

I meant r79895.

Status & tagging log