(As Soon As Reasonable)
On Wed, Dec 24, 2014 at 11:02 AM, Matthew Flaschen mflaschen@wikimedia.org wrote:
...
Fixed the PageTriage bug that was making it unusable (though another one
cropped up soon after), then deployed that ...
enwiki users found another issue with PageTriage: all the dialogs with text fields fail. So it's not completely broken but about half the actions on the page curation toolbar fail. Page curators can't curate over the holidays.
I made an easy fix https://phabricator.wikimedia.org/T78592 and Kaldari graciously +2d it. It seems a safe deploy, but we have no deploys scheduled and no Release Manager around.
In #wikimedia-operations ori commented "Just do it", so I'm exercising judgement here and going for it.
-- =S Page Collaboration team engineer
On Wed, Dec 24, 2014 at 3:56 PM, S Page spage@wikimedia.org wrote:
(As Soon As Reasonable)
enwiki users found another issue with PageTriage: all the dialogs with text
fields fail. So it's not completely broken but about half the actions on the page curation toolbar fail. Page curators can't curate over the holidays.
I made an easy fix https://phabricator.wikimedia.org/T78592 and Kaldari graciously +2d it. It seems a safe deploy, but we have no deploys scheduled and no Release Manager around.
In #wikimedia-operations ori commented "Just do it", so I'm exercising judgement here and going for it.
Thanks S, that was indeed the right call.
I've added Kaldari, Timo, and James to this thread, because I'm speculating now that reenabling jquery.migrate is potentially the quick-and-dirty solution for this class of problem should it keep cropping up over the holidays. Specifically, reverting this: https://gerrit.wikimedia.org/r/#/c/137168/
Ideally, of course, we fix the problem rather than reenabling jquery.migrate, but I'd be very sympathetic to anyone who, when faced with a Christmas day debugging exercise, wants to take a shortcut. I think I'll send a note out to a wider audience to that effect, barring any complaints or alternative suggestions here.
Rob
I'm glad the PageTriage fix worked, thanks for everyone's help.
I ran core 4882249 (before jquery.migrate was deleted) together with PageTriage b60dcf2b (before our JS fixes), and noticed these JQMIGRATE warnings:
*live()* Clicking the page curation toolbar's info button warns JQMIGRATE: jQuery.fn.live() is deprecated but that warning didn't get back to the Collaboration team. (Our first patch fixed this.)
I think this would have contributed to the mw.js.deprecate.jqmigrate.live.count, which graphite suggests was around 2.4K until December 3rd.
Several extensions still have "live(" in their JS in 1.25wmf12: DonationInterface, MoodBar, SemanticMediaWiki, and core's resources/src/mediawiki/mediawiki.htmlform.js
*.attr( 'value' )* PageTriage's deletion dialogs don't warn about using attr( 'value' ), presumably because it can be a legitimate request for the original value of an input field in HTML. (Our second patch fixed this.)
EducationProgram and SemanticForms's use of attr('value') in 1.25wmf12 may be suspect. Something was contributing to the 7k count for mw.js.deprecate.jqmigrate.attr-prop in graphite until Dec 3rd.
*Other warnings*PageTriage is still warning:
JQMIGRATE: jQuery.fn.andSelf() replaced by jQuery.fn.addBack() JQMIGRATE: jQuery.fn.attr('checked') may use property instead of attribute JQMIGRATE: $(html) HTML strings must start with '<' character
are any of these must-fixes? How will we be reminded to fix them if jquery.migrate is turned off?
Ideally the jquery.migrate logging would have warned "Dude, this one's coming from extension PageTriage', I guess that requires fragile AI to walk the stack or DOM looking for the guilty party. I saw lots of JQMigrate warnings in Firebug until it was removed, but it was hard to tell what code was triggering the warning.
I'm not pointing fingers or blaming anyone, this ball was in Collaboration team's court. https://phabricator.wikimedia.org/T85022 is our bug to audit the rest of our extensions.
Cheers, hope y'all had a fine Xmas.
On Wed, Dec 24, 2014 at 5:10 PM, Rob Lanphier robla@wikimedia.org wrote:
On Wed, Dec 24, 2014 at 3:56 PM, S Page spage@wikimedia.org wrote:
(As Soon As Reasonable)
enwiki users found another issue with PageTriage: all the dialogs with
text fields fail. So it's not completely broken but about half the actions on the page curation toolbar fail. Page curators can't curate over the holidays.
I made an easy fix https://phabricator.wikimedia.org/T78592 and Kaldari graciously +2d it. It seems a safe deploy, but we have no deploys scheduled and no Release Manager around.
In #wikimedia-operations ori commented "Just do it", so I'm exercising judgement here and going for it.
Thanks S, that was indeed the right call.
I've added Kaldari, Timo, and James to this thread, because I'm speculating now that reenabling jquery.migrate is potentially the quick-and-dirty solution for this class of problem should it keep cropping up over the holidays. Specifically, reverting this: https://gerrit.wikimedia.org/r/#/c/137168/
Ideally, of course, we fix the problem rather than reenabling jquery.migrate, but I'd be very sympathetic to anyone who, when faced with a Christmas day debugging exercise, wants to take a shortcut. I think I'll send a note out to a wider audience to that effect, barring any complaints or alternative suggestions here.
Rob
On Thu, Dec 25, 2014 at 3:24 AM, S Page spage@wikimedia.org wrote:
[..]
*Other warnings*PageTriage is still warning:
JQMIGRATE: jQuery.fn.andSelf() replaced by jQuery.fn.addBack() JQMIGRATE: jQuery.fn.attr('checked') may use property instead of attribute JQMIGRATE: $(html) HTML strings must start with '<' character
are any of these must-fixes? How will we be reminded to fix them if jquery.migrate is turned off?
Ideally the jquery.migrate logging would have warned "Dude, this one's coming from extension PageTriage', I guess that requires fragile AI to walk the stack or DOM looking for the guilty party. I saw lots of JQMigrate warnings in Firebug until it was removed, but it was hard to tell what code was triggering the warning.
andSelf() has been deprecated but removal has not been announced yet upstream. However we no longer support older jQuery versions (each MediaWiki release ships one version, and that's the one it supports). Nothing is stopping us from migrating calls to addBack().
attr('checked') is a must-fix. Browsers parse attributes html strings. The values are stored in the Node tree attribute objects and can be mutated there. However they are only used to to provide initial values of Node properties. Properties represent live state, attributes do not (except for e.g. css selectors, which apply to tree structure, not Node objects). Older versions of jQuery did counter-intuitive trickery to sometimes return or set values of properties instead of attributes. This has been removed in favour of a .prop('checked') method.
$(html) is a must-fix. $() is a heavily overloaded method (supporting css selector strings, among many other things). To avoid ambiguity and for security reasons, it now only supports valid HTML that starts with a '<' character. If you have other html strings (e.g. starting with text nodes), use $.parseHTML() directly.
-- Krinkle
On 12/24/2014 10:24 PM, S Page wrote:
*PageTriage is still warning:
JQMIGRATE: jQuery.fn.andSelf() replaced by jQuery.fn.addBack()
As I said on Phabricator (https://phabricator.wikimedia.org/T85022#948436), my grepping indicated this was *not* from PageTriage.
Matt Flaschen