Code Review
From MediaWiki.org
For MediaWiki (recent comments | status changes | tags | authors | states | release notes)
![]() First page |
![]() Previous page |
![]() Next page |
![]() Last page |
| Date | Commenter | Revision | Status | Commit summary | Note |
|---|---|---|---|---|---|
| 17:56, 20 November 2009 | Werdna | 58789 | ok | (although, ideally most of this should be done with a pager, and the links, while safe, should really be constructed with the Skin object). | |
| 17:56, 20 November 2009 | Bryan | 58657 | ok | Reviewed by Werdna. | |
| 17:56, 20 November 2009 | Werdna | 58789 | ok | Looks OK | |
| 17:56, 20 November 2009 | Bryan | 58693 | ok | Reviewed by Werdna. | |
| 17:56, 20 November 2009 | Bryan | 58705 | ok | Reviewed by Werdna. | |
| 17:46, 20 November 2009 | Werdna | 58778 | ok | Looks fine | |
| 17:46, 20 November 2009 | Werdna | 58763 | ok | Looks fine. | |
| 17:46, 20 November 2009 | Werdna | 58758 | ok | Looks ok | |
| 17:43, 20 November 2009 | Mormegil | 59152 | new | And you cannot copy&paste <code>context.modules.$toc</code> into <tt>jquery.wikiEditor.toolbar.js</tt>, you need to use <code>$toolbar</code> there. | |
| 17:37, 20 November 2009 | Mormegil | 59152 | new | That “*/” forgotten in wikiEditor.css is redundant [http://svn.wikimedia.org/viewvc/mediawiki/trunk/extensions/UsabilityInitiative/css/wikiEditor.css?%721=59151&%722=59152] | |
| 17:29, 20 November 2009 | Trevor Parscal | 59276 | fixme | Oops - yes, ialex has moved it to the right place. | |
| 10:06, 20 November 2009 | Catrope | 59275 | new | <pre> - '<html><head><title>wikiEditor</title></head><body style="margin:0;padding:0;width:100%;height:100%;">' + - '<pre style="margin:0;padding:0;width:100%;height:100%;white-space:pre-wrap;"></pre></body></html>' + '<html><head><title>wikiEditor</title><script>var context = window.parent... | |
| 03:01, 20 November 2009 | ^demon | 59276 | fixme | Should be in /trunk/extensions? | |
| 18:00, 19 November 2009 | Reedy | 59258 | new | Not sure if "$this->watched[$row->page_namespace][$row->page_title] = true;" would be better to be "$this->watched[$row->page_namespace][$row->page_title] = '';" or similar? | |
| 14:17, 19 November 2009 | Catrope | 59196 | ok | Test comment to see if reedy's enotif works. | |
| 12:25, 19 November 2009 | Catrope | 59247 | fixme | The rule is ignored. This is actually a trick the Vector stylesheets use to apply rules for IE6 only: <pre> /* Rules for IE6 */ foo bar { blah: 0; } /* Non-IE6 browsers will parse this rule and let it override the previous one */ foo > bar { blah: 1; } </pre> | |
| 11:47, 19 November 2009 | Werdna | 59247 | fixme | What happens if I do use one in IE6? | |
| 11:47, 19 November 2009 | Catrope | 59247 | fixme | Child selectors don't work in IE6. | |
| 10:18, 19 November 2009 | Catrope | 59221 | new | Adam recombined it for you in r59228. For future reference, you can do this by running make in the UsabilityInitiative directory. Also please update the relevant style versions (done in r59238). | |
| 08:03, 19 November 2009 | Catrope | 58974 | resolved | Agreed. However, we decided that using click instead of mousedown was OK, so that's what we did. | |
| 01:11, 19 November 2009 | Tfinc | 59225 | new | Looks like extra debugging made its way in here. | |
| 00:42, 19 November 2009 | Tim Starling | 58974 | resolved | I think if you have a choice between browser sniffing and totally broken behaviour, you should choose browser sniffing. | |
| 22:11, 18 November 2009 | Nimish Gautam | 59205 | ok | quoted using dbr instead of manual quotes | |
| 21:47, 18 November 2009 | Siebrand | 59216 | new | Has 'descriptionmsg' => 'myextension-desc' but no i18n file and wgExtensionMessagesFiles[] | |
| 18:59, 18 November 2009 | Catrope | 58974 | resolved | Changed back to using click() in r59202 | |
| 18:55, 18 November 2009 | Shuhari | 57425 | ok | This revision was reviewed by Werdna at 6:44pm UTC on November 18th before production deployment. | |
| 18:50, 18 November 2009 | Shuhari | 57541 | ok | This revision was reviewed by Werdna at 6:44pm UTC on November 18th before production deployment. | |
| 18:49, 18 November 2009 | Shuhari | 58969 | ok | This revision was reviewed by Werdna at 6:44pm UTC on November 18th before production deployment. | |
| 18:48, 18 November 2009 | Shuhari | 59185 | ok | This revision was reviewed by Werdna at 6:44pm UTC on November 18th before production deployment. | |
| 18:47, 18 November 2009 | Shuhari | 59191 | ok | This revision was reviewed by Werdna at 6:44pm UTC on November 18th before production deployment. | |
| 11:08, 18 November 2009 | Catrope | 59185 | ok | Yes, I had Andrew and Adam test it. | |
| 11:08, 18 November 2009 | Catrope | 58974 | resolved | That's not intended, no, but I don't think it's a big deal, or that we can cleanly solve this without browser sniffing. The problem would go away if we went back to using click(), but we're using mousedown() for a reason. I'll discuss this with Trevor. | |
| 11:05, 18 November 2009 | Catrope | 58953 | resolved | Yes, I fixed it in those revs, and I had others test it. I downloaded VirtualBox yesterday and I'm gonna play around with it today so I'll (hopefully) finally have IE to test on. Removing fixme as this has been tested by other people, and you haven't explicitly said it didn't work for you (after r5... | |
| 11:01, 18 November 2009 | Catrope | 58873 | fixme | Stripping the leading whitespaces doesn't happen for me on the deployment candidate; it could be and probably is a bug with the magic iframe, which was introduced later. The bug this is meant to fix is that when you accidentally select the newline before or after the heading along with the heading ... | |
| 04:09, 18 November 2009 | Tim Starling | 59185 | ok | Is it tested? | |
| 02:56, 18 November 2009 | Tim Starling | 59048 | fixme | I'm disabling this extension on Wikimedia until the correct SQL escaping functions are used. | |
| 02:31, 18 November 2009 | Tim Starling | 59048 | fixme | This file is wrong in lots of different ways. I'll have to write a full review. | |
| 02:22, 18 November 2009 | Tim Starling | 58974 | resolved | Is it intended that this will cause a middle-button click to be treated the same as a left-button click in FF? | |
| 02:21, 18 November 2009 | Tim Starling | 58972 | resolved | Fixme as per r58953. | |
| 01:33, 18 November 2009 | Tim Starling | 58953 | resolved | Plainly broken, I assume you're hoping this was fixed in r58973 and r58974. I assume the bug 21050 component is also untested. Is it too much to ask that you test your changes before commit? Do we need to sponsor you a Windows license or something? Marking as "fixme" pending proper testing. | |
| 01:02, 18 November 2009 | Tim Starling | 58873 | fixme | It doesn't work for me, in FF 3.5. If I select a heading with spaces at the start of the line, and change it to a different level heading, the spaces at the start of the line disappear, presumably folding into the line break after the <br/>. Note that a heading with spaces preceding is not a... | |
| 00:26, 18 November 2009 | Tim Starling | 58570 | reverted | Roan tells me it is to override a rule from [[:de:MediaWiki:Common.css]]. I have now fixed that page, so this change can be reverted. | |
| 00:15, 18 November 2009 | Tim Starling | 58570 | reverted | Can you explain this? | |
| 22:47, 17 November 2009 | Tim Starling | 58646 | fixme | https://bugzilla.wikimedia.org/show_bug.cgi?id=20554#c5 | |
| 10:46, 17 November 2009 | MaxSem | 59145 | new | Whoopsie. Gotta tweak the checker to test only modified files quickly, otherwise it will be too slow to run it at all times, like in this case. | |
| 13:46, 16 November 2009 | IAlex | 59123 | reverted | Reverted in r59131. | |
| 13:29, 16 November 2009 | IAlex | 59123 | reverted | This produces the following warnings (on translatewiki.net): <pre> [16-Nov-2009 13:09:23] PHP Notice: Undefined property: stdClass::$user_password in /var/www/w/includes/User.php on line 972 [16-Nov-2009 13:09:23] PHP Notice: Undefined property: stdClass::$user_newpassword in /var/www/w/includes... | |
| 13:26, 16 November 2009 | Werdna | 58633 | fixme | Resolution in r58816. | |
| 00:34, 16 November 2009 | Simetrical | 59024 | new | I meant: ". . . will be an HTML processor . . ." | |
| 00:33, 16 November 2009 | Simetrical | 59024 | new | Some more thoughts: * I still don't like the script filtering thing. It gives a false sense of security at best, and is WTFish at worst. * If xml:lang="" is used, it's certainly only valid if it matches lang="" on the same attribute. So why not only allow lang="", and just spit out a matching xml... |
![]() First page |
![]() Previous page |
![]() Next page |
![]() Last page |



