Jump to content

Architecture meetings/RFC review 2014-03-05

From mediawiki.org

21:00 UTC, March 5th, at #wikimedia-office connect.

Requests for Comment to review

[edit]

Two at most, so we can go in depth.

  1. Passwords
  2. TitleValue
  3. Inline diffs

Summary and logs

[edit]

Meeting summary

[edit]
  • TitleValue (sumanah, 21:25:36)
    • LINK: https://www.mediawiki.org/wiki/Requests_for_comment/TitleValue (sumanah, 21:25:48)
    • LINK: https://gerrit.wikimedia.org/r/106517 new version of the TitleValue patch (sumanah, 21:26:13)
    • I asked DanielK_WMDE just before this meeting what he wanted, and he said he wants people to review that patch & ideally merge it :) <DanielK_WMDE> it would be useful to also agree on next steps (refactor more special pages? or try an api module or two? or look into factoring mroe stuff out of Title?) (sumanah, 21:26:46)
    • from Daniel's email on 26 Feb "I have tried to address several issues with the previous proposal, and reduce the complexity of the proposal. I have also tried to adjust the service interfaces to make migration easier." (sumanah, 21:28:12)
    • ACTION: anomie review the changeset (sumanah, 21:39:15)
    • we all want more testing, test cases for the current behavior of Title (sumanah, 21:40:07)
    • IDEA: <hoo> DanielK_WMDE: I'm not a fan of having the special page changes in the same change set (DanielK_WMDE, 21:42:00)
  • Inline diffs RFC (sumanah, 21:46:48)
    • LINK: https://www.mediawiki.org/wiki/Requests_for_comment/Inline_diffs (sumanah, 21:46:56)
    • LINK: https://www.mediawiki.org/wiki/Accessibility_guide_for_developers has some useful links re colours (sumanah, 21:49:02)
    • <moizsyed> i think this is a good idea, keeps the experience cohesive across platforms, also other major platforms on the web where people are used to "diff views" like github have not primarily moved on to a single inline diff view (sumanah, 21:49:19)
    • some concern about colours :) (sumanah, 21:49:36)
    • IDEA: need to device a nice mode-switching UI (keep it inline, avoid having to go to special:preferences) (brion, 21:50:20)
    • LINK: https://en.m.wikipedia.org/wiki/Special:MobileDiff/595151209...595152100 example diff from our current mobile view (sumanah, 21:52:06)
    • IDEA: bawolff reccomends we ask real non-technical users and see what they think (sumanah, 21:53:38)
    • IDEA: check for user studies or similar data about how people actually like to interact with diffs, among Wikimedia users and elsewhere in collaborative writing workflows (sumanah, 21:53:55)
    • discussion and lack of agreement on whether to just turn this on, make it a beta feature, make it a pref, make it an option on the diff page, or something else (sumanah, 22:01:11)
    • LINK: https://en.m.wikipedia.org/wiki/Special:MobileDiff/595047902...595151972 longer diff, possibly confusing to the user (per quiddity) (sumanah, 22:01:47)
    • ACTION: MaxSem to add mode switching UI details to RFC (TimStarling, 22:04:22)
    • Additional Action from right after the meeting: Dan Garry is going to look into whether we want to make this a beta feature, a preference, an option on the diff page, or something else


Meeting ended at 22:09:13 UTC.


Action items

[edit]


Action items, by person

[edit]


Full log

[edit]
Meeting logs
20:59:51 <sumanah> #startmeeting RFC review TitleValue and password strength
20:59:51 <wm-labs-meetbot> Meeting started Wed Mar  5 20:59:51 2014 UTC and is due to finish in 60 minutes.  The chair is sumanah. Information about MeetBot at http://wiki.debian.org/MeetBot.
20:59:51 <wm-labs-meetbot> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
20:59:51 <wm-labs-meetbot> The meeting name has been set to 'rfc_review_titlevalue_and_password_strength'
21:00:07 <sumanah> #chair brion Tim-away
21:00:07 <wm-labs-meetbot> Current chairs: Tim-away brion sumanah
21:00:23 <sumanah> #link https://www.mediawiki.org/wiki/Architecture_meetings/RFC_review_2014-03-05
21:00:32 <sumanah> csteipp: do you mind going first?
21:00:44 <csteipp> Sure...
21:00:48 <sumanah> ok
21:00:56 <sumanah> #topic Password strength
21:01:10 <csteipp> Last meeting (can't find link) we said we wanted more data on passwords. I updated the RFC with those.
21:01:25 <csteipp> https://www.mediawiki.org/wiki/Requests_for_comment/Passwords#Threats
21:01:41 <sumanah> #info https://www.mediawiki.org/wiki/Architecture_meetings/RFC_review_2014-02-05 - was #action csteipp will research and update the rfc with estimate for online attacks to compromise accounts to get autoconfirmed access.
21:02:42 * brion reads
21:02:47 <MaxSem> csteipp, 6 chars passwords are going to piss a lot of users off
21:03:12 <sumanah> I asked csteipp what he'd like from the next 1/2 hour and he said <csteipp> sumanah: Yeah, a decision would be nice, or at least, "this is the right direction, now we do X"
21:03:31 <DanielK_WMDE> MaxSem: really? and i thought i was lazy...
21:03:35 <csteipp> I was pulling for 4, and the realized how quick that was to brute force
21:03:48 <csteipp> s/the/then/
21:04:04 <MaxSem> DanielK_WMDE, mine's much longer;)
21:04:13 <sumanah> #idea is this something where the architects are interested in delegating this decision to a particular decider?
21:04:14 <parent5446> I still can't believe we haven't already increased the minimum to 6.
21:04:37 <brion> i'd like to go ahead and do that increase bump
21:04:45 <brion> does that still need a software fix to allow old shorter passwords?
21:04:53 <manybubbles> csteipp: so long as we don't require three different kinds of special characters but not _s.  no, not _s, then I'm in favor
21:05:02 <csteipp> brion: Yeah, we need a small patch to allow the old passwords through
21:05:06 <csteipp> But it's small
21:05:14 <brion> yeah, a min length and a suggested strength meter should be fine i think
21:05:23 <brion> no need to demand specific upper/lower/special/number i hope
21:05:27 <brion> csteipp: great
21:05:42 <csteipp> Yeah, there are no complexity requirements
21:05:47 <brion> any objections to moving ahead in that direction?
21:05:47 <MaxSem> csteipp, clarification: you're looking to bump it on WMF only, right?
21:05:57 <csteipp> The proposal did consider having a list of forbidden passwords
21:06:04 <DanielK_WMDE> csteipp: "they can brute force about 43,000 passwords per month" <--- i didn't quite follow the math, but that's password *attempts*, not sucessfully cracked passwords, right?
21:06:06 <csteipp> MaxSem: yES
21:06:16 <csteipp> DanielK_WMDE: Yes, attempts
21:06:19 <brion> MaxSem: i would recommend bumping the minimum as a default as well personally
21:06:24 <DanielK_WMDE> csteipp: that should be clarified in the text
21:06:43 <DanielK_WMDE> "brute forcing a password" to me means actually getting access
21:06:55 <sumanah> #info I have a question about what's holding up https://gerrit.wikimedia.org/r/#/c/77645/ ("After Gerrit change 77645 is merged, we will be able to force a password reset without locking users out of their accounts. " - the RFC)
21:07:06 <csteipp> #action csteipp update language to clarify attempts vs breaks
21:07:23 <TimStarling> on non-WMF wikis, you potentially need longer passwords
21:07:48 <TimStarling> because the login throttle only works if memcached is enabled, right?
21:07:57 <brion> are we still bike shedding on the basing algo? or is anything else holding it up?
21:08:02 <csteipp> yep
21:08:09 <brion> TimStarling: that .... could probably be fixed. good point to bring up tho
21:08:14 <duh> yeah, you need a main cache to be set
21:08:16 <brion> *hasing
21:08:17 <brion> *hashing
21:08:21 <DanielK_WMDE> what'S the rationale for including the IP in the throttle key? is that to avoid the legit user being locked out by someone constantly hitting their accounts with login attempts?
21:08:31 <brion> DanielK_WMDE: yes
21:08:35 <csteipp> brion: Nope, the hashing is settled. Just waiting on a couple minor tweaks form parent5446
21:08:38 <DanielK_WMDE> perhaps there should be two separate throttles, one per ip and one per account.
21:08:40 <brion> if we key only on user then it's trivial to DoS someone's account
21:08:47 <sumanah> #idea <DanielK_WMDE> perhaps there should be two separate throttles, one per ip and one per account.
21:09:08 <parent5446> Gonna work on that next week over spring break.
21:09:19 <sumanah> #idea <TimStarling> on non-WMF wikis, you potentially need longer passwords     because the login throttle only works if memcached is enabled, right?
21:09:20 <brion> #info password fail throttle currently relies on memcached, needs to be fixed to work on more 3rd-party sites
21:09:21 <csteipp> parent5446: Thanks!
21:09:47 <DanielK_WMDE> #help brion clarify the rationale behind the current setup/implementation
21:09:58 <TimStarling> anyway, the proposal is pretty simple and can presumably be approved
21:10:03 <TimStarling> if there are no objections?
21:10:18 <brion> so the throttle we have in place is fairly old and primitive. it doesn't take attackers with multiple IPs into account well
21:10:21 <brion> TimStarling: i'm in favor
21:10:28 <sumanah> TimStarling: brion if you're ok with it then go ahead & #agreed  the approval of the increase to a 6-byte min
21:10:37 <MaxSem> TimStarling, I wonder if we should just make caching default to CACHE_ANYTHING instead of CACHE_NONE....
21:10:40 <TimStarling> i.e. increase $wgMinimalPasswordLength to 6 on WMF, and do the patch to login to allow old short passwords to be used
21:10:44 <sumanah> do you want to check in with mark on this?
21:11:09 <TimStarling> MaxSem: yes, I think we can do that
21:11:17 <DanielK_WMDE> it might make sense to require stronger passwords for accounts with more privileges
21:11:20 <brion> MaxSem: agreed on that
21:11:23 <DanielK_WMDE> i vaguely remember that being discussed before
21:11:24 <TimStarling> I think at the time it was introduced, SqlBagOStuff didn't support expiries, but now it does
21:11:35 <duh> if people have a <6 password, will we continute to allow that until they want to change it, or are we goign to force them to change it upon next login?
21:11:40 <brion> DanielK_WMDE: that's a possibility but let's leave that back for later, once we have strength calculations
21:11:41 <sumanah> DanielK_WMDE: that's https://bugzilla.wikimedia.org/show_bug.cgi?id=44788
21:11:49 <csteipp> DanielK_WMDE: That has been suggested several times. I see that as probably the next step
21:11:53 <TimStarling> I mean, it had expiration, but it wasn't checked on fetch, only randomly on purge
21:11:53 <brion> #agreed increase $wgMinimalPasswordLength to 6 on WMF, and do the patch to login to allow old short passwords
21:12:00 <DanielK_WMDE> ok, thanks.
21:12:02 <sumanah> duh: that's why I brought up the changeset that's awaiting merge...
21:12:11 <brion> #idea need to work out whether we just allow the old passwords or force them to update
21:12:54 <csteipp> duh: Either is fine. I would probably make it a config flag
21:13:05 <TimStarling> csteipp: has anything been done on that forced password change feature we've talked about?
21:13:14 <duh> csteipp: well, what would you set it to on WMF wikis? ;)
21:13:14 <csteipp> TimStarling: Merged
21:13:24 <MaxSem> A butthurt expect I...:P
21:13:33 <TimStarling> right
21:14:00 <sumanah> wait, csteipp, I thought https://gerrit.wikimedia.org/r/#/c/77645/ was a necessary prereq to the forced reset? it's not merged yet
21:14:18 <TimStarling> user_password_expires
21:14:25 <hoo> sumanah: No, unrelated to expiring
21:14:35 <hoo> thanks for the email reminder, btw
21:14:36 <csteipp> what hoo said :)
21:14:46 <sumanah> hoo: you are welcome :)
21:14:52 <csteipp> Hashing and password API will make future development easier
21:15:18 <csteipp> But I did the password expiration in a separate patch, since I want to get rid of our config hack from the Oct incident.
21:16:03 <sumanah> ah ok. csteipp https://gerrit.wikimedia.org/r/#/q/message:user_password_expires,n,z doesn't find it - help? :)
21:16:19 <csteipp> https://gerrit.wikimedia.org/r/#/c/92037/
21:16:28 <TimStarling> forced password reset due to a short password would be basically the same as expiration, just $this->mAbortLoginErrorMsg would need to be different
21:17:21 <csteipp> Yeah, the forced reset flow from https://gerrit.wikimedia.org/r/#/c/92037/ will make that pretty easy, if we want to do it that way
21:17:25 <sumanah> #info https://gerrit.wikimedia.org/r/#/c/92037/ merged "Add functionality to expire users' passwords"
21:18:02 * sumanah had to add "status:merged" to gerrit search because by default it only searches open changesets. got it
21:18:41 <sumanah> ok, so next actions - should csteipp go ahead and implement the increase in minimum bytelength? and turn the user group-level requirements into a new RFC?
21:19:10 <brion> i say yes :)
21:19:17 <hoo> Definitely
21:19:37 <sumanah> #action csteipp go ahead and implement the increase in minimum bytelength
21:19:39 <duh> +1
21:20:04 <sumanah> #action csteipp turn the user group-level requirements into a new RFC  https://www.mediawiki.org/wiki/Talk:Requests_for_comment/Passwords#Create_new_password_requirements_for_accounts_with_advanced_user_rights  https://bugzilla.wikimedia.org/show_bug.cgi?id=44788
21:20:11 <duh> I would also advocate forcing everyone to make them longer now rather than people finding out later
21:20:45 <sumanah> #action csteipp coordinate with the community liaisons, Legal & Community Advocacy, et alia to communicate out about this change before flipping the switch
21:20:56 <sumanah> Deskana: I presume you'd be in on that as well?
21:21:13 <hoo> I said this before, but I'd also which to have maint. script which tests all our sysops/oversights and checkuser accounts against a list of most commonly used passwords so that we can take action
21:21:14 <DanielK_WMDE> stupid question - am i right in assuming that there is no way to know the password's length until the user enters it in order to log in?
21:21:21 <Deskana> sumanah: Kinda busy. WIll have to read up later and reply later.
21:21:21 <hoo> DanielK_WMDE: yep
21:21:30 <sumanah> got it Dan
21:21:30 <DanielK_WMDE> good good :)
21:21:32 <hoo> s/which/wish/
21:21:33 <csteipp> DanielK_WMDE: Short of brute forcing the db, yes
21:21:46 <DanielK_WMDE> just making sure...
21:22:13 <sumanah> #link https://www.mediawiki.org/wiki/Requests_for_comment/Passwords
21:22:16 <duh> DanielK_WMDE: yeah, so we could force the reset on the next login, which is what was done during the data leak
21:22:35 <sumanah> what about the strength meter? are we going ahead with approving that today also?
21:22:51 <duh> I'm not a fan of strength meters
21:22:53 <sumanah> #idea <MaxSem> TimStarling, I wonder if we should just make caching default to CACHE_ANYTHING instead of CACHE_NONE....
21:23:00 <hoo> I'm slightly worried by the fact that we have thousands of accounts being able to edit JS which are partially inactive for years and might be very easy to crack by using commons passwords, like "password" or "123456"
21:23:11 <MaxSem> we don't have a proposed strength meter yet
21:23:25 <MaxSem> merely discuss if we need it
21:23:36 <duh> hoo: so why don't we just run a password cracker across all admin accounts?
21:24:03 <hoo> duh: Not a real cracker, but someting which tests a cheap list of passwords, that's basically my idea, yes
21:24:14 <hoo> to get the low hanging fruits out of them
21:24:15 <TimStarling> right, we're not approving a strength meter, just the section "Proposed change" in the RFC
21:24:17 <duh> sure
21:24:17 <csteipp> hoo: As part of the strong passwords for admin accounts, I think I'll explicitely include a section where we audit and force reset passwords that we can crack. I'm not sure if that will last, but I think that's reasonable for us.
21:24:31 <duh> +1 to that
21:24:42 <hoo> +2
21:24:47 <manybubbles> sounds reasonable
21:25:09 <TimStarling> should we move on to the next RFC?
21:25:12 <sumanah> ok, csteipp do you need any particular thoughts on "Create a password strength indicator", "Require more complex passwords", or "Password expiration" now, or shall we move on?
21:25:30 <csteipp> sumanah: I think we can move on. Thank!
21:25:36 <sumanah> #topic TitleValue
21:25:48 <sumanah> #link https://www.mediawiki.org/wiki/Requests_for_comment/TitleValue
21:26:13 <sumanah> #link https://gerrit.wikimedia.org/r/106517 new version of the TitleValue patch
21:26:42 <DanielK_WMDE> right. so, for anyone who hasn't looked at TitleValue recently: I have overhault it about a week ago
21:26:43 <manybubbles> I'm a fan of at least this incarnation
21:26:46 <sumanah> #info I asked DanielK_WMDE just before this meeting what he wanted, and he said he wants people to review that patch & ideally merge it :)  <DanielK_WMDE> it would be useful to also agree on next steps (refactor more special pages? or try an api module or two? or look into factoring mroe stuff out of Title?)
21:27:43 <DanielK_WMDE> right. so, first off, i'd like to know if there are any issues left, and if not, what that path is to getting this merged.
21:28:12 <sumanah> #info from Daniel's email on 26 Feb "I have tried to address several issues with the previous proposal, and reduce the complexity of the proposal. I have also tried to adjust the service interfaces to make migration easier."
21:28:15 <DanielK_WMDE> i think i was able to smoothen out quite a few issues over the last weeks.
21:28:27 <DanielK_WMDE> sumanah: you are just faster than me :P
21:28:42 <sumanah> DanielK_WMDE: just playin' backup :)
21:29:27 <TimStarling> anomie: have you looked at the last patchset?
21:29:47 <DanielK_WMDE> he commented a few minutes ago.
21:30:09 <anomie> I glanced at it just before this meeting, haven't had a chance to look if the functionality issues from PS13 were all addressed or not
21:30:58 <TimStarling> I see Chad gave a +1 to PS14
21:31:29 <manybubbles> are we to the point where we just need to make sure it is 100% transparent and rebased and stuff?
21:31:34 <manybubbles> or is there something else
21:31:57 <manybubbles> by transparent I mean backwards compatible/won't break anything
21:32:15 <hoo> IMO were good enough to go sometime soon
21:32:37 <TimStarling> I see you figured out what to call something that is both a parser and a formatter
21:32:54 <DanielK_WMDE> well, "codec" isn't ideal, but i couldn't think of anything better
21:33:18 <manybubbles> I can think of lots of worse names as well, but nothing better
21:33:24 <DanielK_WMDE> the class could be split, but since both need the same knowledge (namespace names, in particular) it seems reasonable to implement both interfaces in a single class
21:33:38 <DanielK_WMDE> "Forser" :)
21:34:53 <DanielK_WMDE> note that i reverted my attempt to refactor secureAndSplit (now i just moved it and polished it a bit)
21:35:02 <DanielK_WMDE> i also got rid of the "single function" interfaces
21:35:26 <DanielK_WMDE> hey ^d
21:35:35 <^d> ohai
21:36:30 <csteipp> DanielK_WMDE: I just noticed you did that-- so that's just a straight copy of the current version?
21:36:40 <csteipp> SecureAndSplit, that is
21:37:13 <DanielK_WMDE> csteipp: mostly. it replaces access to member variables with array fields, and access to globals with member variables.
21:37:16 <hoo> AFAIK that's mostly true
21:37:19 <DanielK_WMDE> that's probably all i changed.
21:38:05 <DanielK_WMDE> the way it's now still needs improvement - but before i mess with that, i want a LOT more test cases for the current behavior of Title, so i don't break anything
21:38:11 <MaxSem> TimStarling & brion, https://gerrit.wikimedia.org/r/117091 :)
21:38:15 <brion> so is there anything holding us up from a merge, or do we want to do some more looking over / testing / etc first?
21:38:26 * brion assumes testing is wise :D
21:38:33 <TimStarling> brion: I'd like to see a +1 from anomie at least
21:38:40 <DanielK_WMDE> if more checking / testing is needed, someone should commit to doing it
21:38:52 <hoo> DanielK_WMDE: I'm not a fan of having the special page changes in the same change set
21:38:55 <DanielK_WMDE> otherwise we'll meet again in 4 weeks, no wiser than we are now
21:39:05 <brion> *nod*
21:39:13 <TimStarling> but I think we can approve the RFC at this point
21:39:15 <sumanah> #action anomie review the changeset
21:39:19 <DanielK_WMDE> hoo: true. i wanted to make sure it is clear how the new class should be used.
21:39:31 <brion> sensible
21:39:49 <DanielK_WMDE> but it would be cleaner to have the special page stuff in a separate change set. if there is agreement that that would be an improvement, i'll factor that out
21:39:52 <DanielK_WMDE> it's easy enough
21:39:57 <hoo> please do
21:39:59 <TimStarling> since the recent criticisms have been about details, not about the principle
21:40:07 <sumanah> #info we all want more testing, test cases for the current behavior of Title
21:40:36 <brion> so any objections to a accepting the RFC and committing to getting the review/merge done in next few weeks?
21:40:36 <csteipp> I actually liked having the special pages in there... but yeah, either way
21:40:39 <DanielK_WMDE> sumanah: these test cases are needed mostly when messing with the current parsing/splitting code. not neccessary for this patch set, i think
21:40:50 <sumanah> yes, for the future, understood
21:40:53 <DanielK_WMDE> though of course, always nice to have. but not easy to write/come up with
21:41:07 * sumanah defers to Tim/Brion on stating "#agreed"
21:41:43 <sumanah> MaxSem: it looks like we might have time today to talk about inline diffs, contrary to my predictions!
21:41:51 <MaxSem> wee
21:42:00 <DanielK_WMDE> #idea <hoo> DanielK_WMDE: I'm not a fan of having the special page changes in the same change set
21:43:02 <hoo> So, if Anomie +1 this and DanielK_WMDE is going to remove the special page changes, we are good to go? If so I might jump in to actually press the button in the end
21:43:24 <DanielK_WMDE> i'd like to know whether others also think the special page stuff should be factored out
21:43:29 <TimStarling> #agreed TitleValue RFC accepted, merge proposed change after code review
21:43:43 * anomie will hopefully find time to re-review next week
21:43:58 <DanielK_WMDE> \o/
21:44:08 * DanielK_WMDE does a happy dance
21:44:10 <manybubbles> DanielK_WMDE: I liked them in there as an example of why you'd want to do this.  Maybe less clean but more compelling.  I'll live with not having them if it makes everyone happier.
21:44:12 <addshore> =]
21:44:16 <sumanah> ok, do we need any more discussion on this or can we move on to https://www.mediawiki.org/wiki/Requests_for_comment/Inline_diffs ?
21:44:54 <DanielK_WMDE> i'll leave them in for now, unless hoo gives a -1 for that :)
21:45:25 <hoo> Wil have another look at them... if they have the potential to break, I'll -1... keep changes atomic
21:45:59 <DanielK_WMDE> sure. splitting off two files is easy enough
21:46:23 <DanielK_WMDE> thanks everyone for your encouragement & criticism!
21:46:35 * DanielK_WMDE is going to pack up and go home now
21:46:40 <manybubbles> thanks for doing it
21:46:42 <manybubbles> good night
21:46:43 <brion> whee :D
21:46:48 <sumanah> #topic Inline diffs RFC
21:46:56 <sumanah> #link https://www.mediawiki.org/wiki/Requests_for_comment/Inline_diffs
21:47:03 <sumanah> "I propose to integrate inline (one-column) diffs from MobileFrontend into MediaWiki core. Mobile screens are smaller so standard side-by-side diffs aren't good. To address this, the mobile team developed a new diff mode which we feel might be useful for desktop too."
21:47:25 <sumanah> moizsyed: you are a designer and I believe you have thoughts on this that you shared in the design channel? :)
21:48:06 <TimStarling> MaxSem: there would be a link or some other UI to switch between the two modes on desktop?
21:48:13 <brion> i really like the inline diff mode, though the colors may need to be changed (-> accessibility bikeshed for later)
21:48:18 <DanielK_WMDE> so, "inline" doesn't inlined with page content, but just "unified"-style instead of side by side?
21:48:27 <TimStarling> ohnoes, green/red
21:48:50 <TimStarling> after the massive thread about diff colours when they didn't even carry semantic information?
21:48:56 <brion> DanielK_WMDE: like this: https://en.m.wikipedia.org/wiki/Special:MobileDiff/595151209...595152100
21:49:02 <sumanah> https://www.mediawiki.org/wiki/Accessibility_guide_for_developers has some useful links re colours
21:49:08 <moizsyed> i think this is a good idea, keeps the experience cohesive across platforms, also other major platforms on the web where people are used to "diff views" like github have not primarily moved on to a single inline diff view
21:49:14 <MaxSem> DanielK_WMDE, I called them inline to prevent confusion with diff -u
21:49:18 <hoo> I think this is not about colors right now...
21:49:19 <sumanah> #info <moizsyed> i think this is a good idea, keeps the experience cohesive across platforms, also other major platforms on the web where people are used to "diff views" like github have not primarily moved on to a single inline diff view
21:49:36 <sumanah> #info some concern about colours :)
21:49:40 <rdwrer> I think he may have meant "have now primarily"
21:50:16 <MaxSem> "This is a mobile design which is not necessary to be ported to desktop - however our design team claims that it covers most cases of colorblindness. Yellow and blue covers more cases however unlike side-by-side diffs these colors don't give you idea what was removed and what added. We tried pluses and minuses, looked ugly. Probably, removals can be stricken out. Max Semenik (talk) 22:53, 13 Febru
21:50:20 <brion> #idea need to device a nice mode-switching UI (keep it inline, avoid having to go to special:preferences)
21:50:30 <rdwrer> Wait.
21:50:40 <rdwrer> The styling can be different easily, that's a different matter
21:50:49 <brion> yes. let's NOT BIKESHED NOW on colors
21:50:55 <rdwrer> We can just have semantic information like TimStarling wants anyway
21:50:56 <brion> can bike shed later when we're not on the clock
21:51:00 <moizsyed> ideally we shouldn't use colors only to denote meaning, we should have some other visual indicator too
21:51:15 <rdwrer> Then we can use the classes or whatever to style
21:51:16 <brion> s/colors/colors and styles/
21:51:19 <moizsyed> so that is an issue we should think about
21:51:28 <MaxSem> underline addtions, strike out removals
21:51:49 <bawolff> Our users arent programmers, they might not be as accepting as git hub (or maybe they might, i really have no idea)
21:51:51 <manybubbles> something like that
21:51:53 <MaxSem> but indeed, let's not bikeshed
21:52:06 <sumanah> #link https://en.m.wikipedia.org/wiki/Special:MobileDiff/595151209...595152100 example diff from our current mobile view
21:52:13 <anomie> I don't know about the comparison to github, that's more of a diff -u style than a dwdiff style as is being discussed here.
21:52:20 <MaxSem> One thing I really wanted input on is: "Current mobile designs don't show numbers of modified lines, however the diffs contain everything for core to display them, for example: <div class="mw-diff-inline-header"><!-- LINES 123,456 --></div>. How exactly should line numbers look like? My current idea is "Lines 123/456" and it looks ugly."
21:52:48 <brion> imho line numbers are not very useful but i could be wrong
21:53:02 * bawolff reccomends we ask real non-technical users and see what they think
21:53:06 <brion> true
21:53:17 <MaxSem> how?
21:53:24 <sumanah> moizsyed: do you know whether we have user studies or similar data about how people actually like to interact with diffs, among Wikimedia users and elsewhere in collaborative writing workflows?
21:53:25 <TimStarling> I am fine with it being merged into core if there is a UI for it, or a second user apart from MobileFrontend
21:53:28 <rdwrer> A VPT thread maybe
21:53:35 <anomie> Gerrit does the same thing for its "unified" style: diff -u with some extra highlighting, not dwdiff.
21:53:38 <sumanah> #idea bawolff reccomends we ask real non-technical users and see what they think
21:53:55 <sumanah> #idea check for user studies or similar data about how people actually like to interact with diffs, among Wikimedia users and elsewhere in collaborative writing workflows
21:54:40 <TimStarling> it's a small change
21:54:46 <brion> so i'd also like to see this merged. it can be opt-in, easy to switch, and even disable able as a config switch if we're paranoid
21:54:55 <sumanah> beta feature?
21:55:02 <TimStarling> IMHO this is a transitional feature on the way to HTML diffs
21:55:05 <brion> hmm, beta features is a possibility too
21:55:13 <TimStarling> you know, like the HTML diffs we had in like 2007?
21:55:13 <moizsyed> quora is another good example, where the audience is not programmers only: http://www.quora.com/I-like-my-job-and-boss-a-lot-but-I-am-underpaid-I-received-an-offer-from-a-competing-firm-for-30-more-How-should-I-use-this-to-increase-my-current-pay-given-that-I-have-no-intention-to-quit-and-dont-want-to-harm-my-current-relationship-with-my-manager/log
21:55:24 <sumanah> this is enough of a UI change that I vote for it to be a beta feature
21:55:29 <brion> TimStarling: yeah those were neat in theory but never quite came together
21:55:30 <sumanah> (for now of course)
21:55:37 <MaxSem> HTML diffs failed because they required too much complexity
21:55:42 <bawolff> I like beta feature idea
21:55:45 <YuviPanda> +1 to beta feature
21:55:50 <brion> my recommendation is to put the new-style diff generation in core, then use it from both MobileFrontend and BetaFeatures ?
21:55:58 <brion> or ..... i just don't want to duplicate it
21:56:02 <TimStarling> too much complexity for the volunteer who was working on them at the time
21:56:59 <anomie> Putting my enwiki user hat on for a moment: It'd be a nice option, but I personally wouldn't like it as the default. Possibly because I'm too used to the current style.
21:57:14 <TimStarling> making it a beta feature implies that it will replace the current diff formatting, right?
21:57:24 <MaxSem> definitely not as a default for everyone:)
21:57:38 <brion> hmmmm
21:57:42 <moizsyed> we havent run user studies on this and i agree with pushing this as a beta feature because of that
21:58:05 <TimStarling> I think it should be a link on the diff page
21:58:12 <TimStarling> or an option in a preferences popover
21:58:17 <TimStarling> not a beta feature
21:58:19 <brion> i like link on the diff page too
21:58:33 <brion> we could make enabling the option a beta feature but that feels excessive to me
21:58:33 <anomie> If we have an easy toggle on the diff page, why make it a beta feature? What would it *do* as a beta feature?
21:58:49 <brion> could be that the beta feature "turns it on by default"
21:58:52 <MaxSem> TimStarling, link on diff page can be a beta feature too:)
21:58:54 <brion> and is just a way of advertising it
21:59:03 <MatmaRex> (note that wikEd has had something like this for ages and it's toggled by a link on the regular diffs)
21:59:14 <TimStarling> MaxSem: you know how many links there are on the diff page?
21:59:16 <brion> nice
21:59:21 <TimStarling> imagine if they all had beta feature wrappers
21:59:36 <TimStarling> you know I think the diff page is ugly and cluttered
21:59:44 <TimStarling> I proposed a redesign for it myself, a few years ago
21:59:54 <sumanah> I strongly think that if we're deciding whether something ought to be in beta features, we should have a representative from Product be able to weigh in. Shall I loop them into this discussion and then we can talk more onlist?
21:59:57 <anomie> "turns it on by default" could be a pref, since I hope the current diff will remain as an option. No need for a beta feature to do that. But for advertising purposes... meh.
22:00:00 <moizsyed> having it as a link might lead us to a possibility where people would expect both views... having it as a beta feature hints that this is *a new thing that will replace this old thing*
22:00:17 <TimStarling> actually, I am thinking of the history page, never mind
22:00:32 <TimStarling> well, they are both cluttered, but I only redesigned one of them
22:00:48 <quiddity> As the RfC page says, the larger diffs can be very cryptic/confusing. Should probably link this, as a 2nd meetbot example: https://en.m.wikipedia.org/wiki/Special:MobileDiff/595047902...595151972
22:01:11 <sumanah> #info discussion and lack of agreement on whether to just turn this on, make it a beta feature, make it a pref, make it an option on the diff page, or something else
22:01:47 <sumanah> #link https://en.m.wikipedia.org/wiki/Special:MobileDiff/595047902...595151972 longer diff, possibly confusing to the user (per quiddity)
22:02:05 <brion> i recommend we retool it a little to plan for the mode-switch link, then figure out if we want to do beta feature integration
22:03:07 <TimStarling> switching mode from the diff page itself is not controversial, right?
22:03:20 <brion> seems not too controversial so far :)
22:04:02 <sumanah> (Sorry that we've run over - once we are done with this, I would like to take a few min to get our agenda together for next week)
22:04:03 <MaxSem> I think I know how to make that diff less scary
22:04:22 <TimStarling> #action MaxSem to add mode switching UI details to RFC
22:05:16 <quiddity> Regarding:  <TimStarling> I am fine with it being merged into core if there is a UI for it, or a second user apart from MobileFrontend   <--   There's been a little bit of discussion about potentially using these in Flow, because the diffs in our comment changes are generally of a simple sort (spelling changes, etc).   More research and discussion is needed though.
22:05:44 <TimStarling> quiddity: Flow has editable comments now?
22:05:55 <moizsyed> this is a big change, given how it will be used a few places, we should loop in product into this
22:06:32 <quiddity> Flow comments can always be edited by the original author (if logged-in), and by [configurable variable, currently just sysop].
22:07:04 <quiddity> eg. https://www.mediawiki.org/w/index.php?title=Talk:Flow&topic_newRevision=rq5ggnhkuv7vogqg&topic_oldRevision=rq5gfdhkeqju0av4&workflow=rpx4cdyu8bls80rm&action=compare-post-revisions
22:07:14 <TimStarling> I was told that the reason Flow didn't want to use pages like LQT was because pages imply editability
22:08:32 <sumanah> Deskana: you are a product manager :) I know you're in another meeting just finishing, but wanted to call your attention to the inline diffs question in backscroll here
22:08:51 <sumanah> ok, we're 8 min over, I'd like for us to continue this on the RFC and/or onlist. Any objections?
22:09:01 <quiddity> TimStarling, I'm not a dev, so cannot really answer that. Sorry. :/
22:09:13 <TimStarling> #endmeeting
Retrieved from "https://www.mediawiki.org/w/index.php?title=Architecture_meetings/RFC_review_2014-03-05&oldid=920827"