[00:14:28] android periodic tests have been fairly stable lately. https://integration.wikimedia.org/ci/job/apps-android-wikipedia-periodic-test/ [05:54:16] niedzielski-afk: when you are around, there is a bug in the new alpha preventing logged-in users from editing protected pages even if they have the needed rights. this doesn't happen on mobile web [15:51:17] https://code.facebook.com/posts/1365439333482197/how-we-built-facebook-lite-for-every-android-phone-and-network [15:51:23] "To achieve an extremely byte-efficient wire protocol, instead of using HTTPS, Lite uses a custom message protocol over TLS (directly over TCP). One of the biggest pain points in a 2G network is that establishing a connection can be very slow; it can take multiple seconds. As most Lite traffic flows over a single connection to the backend, this pain point is mitigated in comparison. " [15:53:22] gilles: cool! might be worth sharing on mobile-l [15:53:41] good idea, will do [16:00:57] matanya: thanks! i'll check it out [16:10:25] matanya: is this the same issue as T85528? [16:10:26] T85528: autoconfirmed users unable to edit semiprotected page via mobile - https://phabricator.wikimedia.org/T85528 [16:25:11] bmansurov: got a minute to hangout? [16:25:24] jhobs: sure [16:25:32] tracy? [16:25:40] yeah [17:22:27] bmansurov: left the commit message mostly the same (including the WIP tag) since I wasn't sure if there was more you wanted to add to it [17:22:42] jhobs: cool, thanks. [17:22:51] jhobs: are you planning on adding php tests too? [17:23:25] bmansurov: I don't think so. We'd basically just be testing one function that just checks a config boolean [17:23:35] on an experiment, nonetheless [17:23:41] doesn't really seem very useful to me [17:24:27] ok [17:24:30] bmansurov: unless you (or someone else) sees a great value in it, I think we're fine w/o it [17:24:42] it is just a link, after all [17:24:42] i think so too [17:24:45] cool [17:24:50] I'll comment on the card [17:48:37] jhobs: i just realized that the tests are ignoring the old button now [17:48:49] jhobs: we should make sure that both the old and new button has coverage [17:49:06] jhobs: maybe apply the config to the beta mode and test the new button in beta only [17:49:18] bmansurov: good call, -1 the patch and I'll get it done [18:04:04] niedzielski: it is, thanks [18:05:18] 👍 [18:16:28] etonkovidova: joining standup? [18:17:12] dbrant: not sure -I am in another standup now [18:17:30] etonkovidova: ok, no worries [18:32:04] coreyfloyd: mini standup [18:37:22] dbrant, bearND, niedzielski hmm, am I blind or is Google Play simply slow? I've the second e-mail from a user asking why the beta app requests the "Identity" permission. Have I missed something [18:38:02] coreyfloyd: we wrapped, email an update if you have one? [18:38:14] FlorianSW: you have not! we're requiring a new permission. I've just updated our FAQ with it: https://www.mediawiki.org/wiki/Wikimedia_Apps/Android_FAQ#Security [18:38:38] jdlrobson: should https://phabricator.wikimedia.org/T125329 be in sprint 68? [18:39:19] dr0ptp4kt: no. [18:39:26] jdlrobson: thx [18:39:29] i'm tracking it [18:39:41] jdlrobson: thx. if it should be in a tracking column, do the needful... [18:39:54] mbinder: sorry had my head down [18:40:03] well technically this card is done - there are just disagreements over implementation and whether it should be done. [18:40:14] mbinder: will do [18:40:18] coreyfloyd: look who's sooooooo productive [18:40:19] dbrant: "This permission is only required in Android versions earlier than 6.0 (Marshmallow). For Android 6.0 and above, this permission is not required." that's why I don't see the permission in the alpha app, too (ANdroid 6 device), yes? :D [18:40:19] we may need to do a temporary solution if we can't make progress there [18:40:35] FlorianSW: yep, that's right [18:40:47] dbrant: cool, thanks for the quick info :) [19:00:22] jdlrobson: do you know about the mw-mobilefrontend-leadsection class? [19:00:34] joakino: yeh FlorianSW just added that [19:00:39] jdlrobson: i'm seeing in the mobileformatter tests that it was added even when sectioning is not enabled [19:00:47] oh really? oops [19:01:44] jdlrobson: for example here, only with removeImages the test says that it should have the section wrapper https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/tests/phpunit/MobileFormatterTest.php#L112-L117 [19:02:23] is that how it is supposed to be? does it just wrap the whole thing with the div always? [19:02:59] should I keep that behaviour? [19:03:50] i'd rather not change the tests [19:04:28] joakino: no.. it should probably not do that [19:05:03] wait wait wait [19:05:03] ok, i'll change the tests [19:05:10] i [19:05:15] 'll wait too haha [19:05:25] the wrapping should only happen when the sectio wrapping is enabled [19:05:41] e.g. if enableExpandableSections = false, no changes whatsoever in wrapping [19:05:57] that makes sense for me? The lead section shouldn't be controlled by the expandable section setting. [19:06:13] That's what is expected from scripts, that use the lead section for other things [19:06:19] FlorianSW: so those tests are wrong? [19:06:43] joakino: why? [19:07:08] do even if sectioning is disabled in mobileformatter, we want the wrapping on a div with the lead class [19:07:24] FlorianSW: jdlrobson ^? [19:07:39] FlorianSW: you mean the Skin.js that trys to grab it? [19:07:48] s/trys/tries [19:07:49] Android app: I love love love the Wiktionary define function. Thank you! (and thanks again to FlorianSW for helping me a few weeks back :) [19:08:10] quiddity: hehe, np :P [19:08:40] joakino: yes, I'm currently not sure, if it's Skin.js or Page.js, but the getLeadSection function doesn't have a condition for enabled or diabled expandable sections [19:09:56] ok, i'll emulate the regex behavior and add a fixme or something (no change to tests) [19:11:06] :) [19:13:48] joakino: FlorianSW sounds good [19:14:24] :) (sorry for the aggressive "wait wait wait", wasn't meant to be aggressive :)) [19:15:16] FlorianSW: not aggressive at all! thanks for the insight [19:15:42] *puh* :D [19:29:41] ApiMobileView is funn [19:31:53] bmansurov: See my latest comment. Try updating localStorage [19:32:10] I think it's because I accessed classical chinese [19:32:23] (a variant) [19:59:55] dbrant: I was going to skip Android triage before we changed it's purpose for today. Would you find it valuable for me to attend? [20:01:16] mbinder: it's mostly for engineers; ok to skip [20:06:24] ok [20:19:55] niedzielski: hey! would you have some time to chat about TWN bits? [20:20:17] mhurd: sure i have a meeting in ten minutes but we can chat until then [20:30:28] omw [20:46:56] jhobs: jdlrobson yurik can you guys take a look at https://phabricator.wikimedia.org/T129394 ? [20:55:25] dr0ptp4kt: why is https://phabricator.wikimedia.org/T128214 not in the current sprint? [20:55:54] and why did we push to production stable knowing this? [20:56:43] to be clear that bug is not just about switching between different languages - nirzar was seeing issues where he was bucketed but not shown the overlay which suggests an issue with the bucketing [20:57:37] (if that issue is fixed and we don't plan to fix the A/B test we should decline) [20:57:54] dr0ptp4kt: i dont have Zero setup right now and I've got my hands full with lazy loading images to look at that bug [20:58:22] jdlrobson: i see. let's not do a head-to-head test until we're sure https://phabricator.wikimedia.org/T128214 is in okay shape. [20:58:41] dr0ptp4kt: haven't westarted a head to head test? [21:02:00] jdlrobson: it would appear so. gimme a few minutes to run some queries. it may be simpler to just fix the bug you noted (possibly related) and swat it. [21:02:14] although this seems like maybe there are two different issues [21:02:42] meaning that maybe we just want to roll back the a/b test. +jdlrobson ^ [21:04:50] mhurd: i'm out of my meeting if you want to chat some more. another time's fine too [21:12:15] niedzielski: oh sure! is now good? [21:12:28] mhurd: yep :) [21:13:10] niedzielski: in batcave (that link i gave u) [21:26:29] https://www.irccloud.com/pastebin/MrHAy7Rj/ [21:26:36] mhurd: niedzielski ^ [21:27:23] jdlrobson: yep, looks like there's a legitimate bug in there. i'll work on a patch to be swat'd to dial down or eliminate the head-to-head for now [21:35:47] jdlrobson: i don't seem to witness https://phabricator.wikimedia.org/T128214 with the url in there https://hi.m.wikipedia.org/wiki/%E0%A4%AC%E0%A4%BF%E0%A4%B2%E0%A5%8D%E0%A4%B2%E0%A5%80?mobileaction=beta#/languages [21:35:54] jdlrobson: it shows me the structured overlay as expected [21:36:29] dr0ptp4kt: depends on how you are bucketed [21:37:28] the original issue may be fixed though.. but without a clear fix that would be a guess. [21:38:03] jdlrobson: ok, let's keep it to one variable at a time. let's get https://phabricator.wikimedia.org/T129369 swat'd [21:38:19] jdlrobson: and we can re-examine the data once that's out and schedule a swat for tomorrow morning if needed. ok? [21:39:29] jdlrobson: you want me to create the swat entry for baha's patch and babysit it? [21:39:56] dr0ptp4kt, just saw it, a bit busy at the moment but will try to get to it shortly. jhobs jdlrobson - any updates on this? [21:40:20] yurik, dr0ptp4kt: I'll take a look and see if I can replicate [21:40:32] jhobs, thx :) [21:40:45] jhobs, most likely its a js issue, but it might be the backend too [21:41:00] yurik: I won't be online too much longer though, but I'll comment on the card with what I can find [21:41:08] thx [22:00:53] jdlrobson: you there? [22:01:37] jdlrobson: did overlays get overhauled or something? The Zero error is a "ContentOverlays.extend is not a function" error [22:04:48] dr0ptp4kt, joakino, jdlrobson, bmansurov: I'm gonna pull this into the sprint since it's Unbreak Now https://phabricator.wikimedia.org/T129394 [22:17:11] jdlrobson: i'll watch the swat of baha's patch https://phabricator.wikimedia.org/T129369#2104927 and watch the data as well, so we can decide whether to roll back the head to head test [22:17:28] if the data don't improve, i'll schedule a rollback of the head to head test for tomorrow morning [22:38:23] Thanks dr0ptp4kt i appreciate it [22:39:02] jdlrobson: sure thing. thanks for keeping an eye on things! [22:51:29] jhobs: that changed a long time ago [22:51:38] Please add Zero tests [22:51:51] that would have got picked up by Jenkins when it happened if they'd had them [22:52:03] jdlrobson: I thought they had been added a while ago [22:52:18] yeh that bug has probably been there for 6 weeks now. That's not cool [22:52:39] jdlrobson: so Zero will have to depend on OOjs now then I assume? [22:53:42] it already is. If it's using extend it needs to depend on mobile.oo and use OO.mfExtend [22:54:00] add a qunit test to create a ZeroOverlay [22:54:13] and that will run with MobileFrontend's and stop this happening again [22:54:45] jdlrobson: ok, will fix bug and add a task for the test [23:16:52] and updates on https://phabricator.wikimedia.org/T123549 jhobs? I may have some bandwidth to review this / pair with you / work on this later and I'd really like to see us get this fixed in next train. It should be a 30 min fix. [23:18:32] jdlrobson: had put it on the backburner for the language icon stuff and then the Zero bug came up. I just submitted a patch for the Zero bug though so I'll take a look at it now [23:18:35] jdlrobson, https://gerrit.wikimedia.org/r/#/c/276375 [23:19:06] jhobs, i don't think i'm the best person to review it, so poked jdlrobson [23:19:11] icanhaztest? [23:19:49] jdlrobson: making the task around it now. Figured it'll prob be a next sprint thing [23:20:02] jhobs ergg zero has absolutely no qunit test infrastructure? Sigh. [23:20:15] dr0ptp4kt: if we are going to be supporting this could be possibly use the unstructured sprint to sort this out [23:20:33] i'm concerned about interrupts [23:20:50] jhobs: i can blind merge. Code looks fine but I'm not setup with Zero and I don't plan to setup today. [23:20:54] jdlrobson: sounds worth consideration. maybe that and event logging [23:21:02] dr0ptp4kt: +1 [23:21:10] jdlrobson: is the qunit error in https://gerrit.wikimedia.org/r/#/c/276266/ cause for concern? [23:21:10] can you remember? I have a terrible memory ;-) [23:21:14] jdlrobson: I tested locally and it all looks good. Fixed a minor style bug while I was at it [23:21:25] dr0ptp4kt: looking [23:21:56] dr0ptp4kt: nope.https://integration.wikimedia.org/ci/job/mwext-qunit/12839/consoleFull [23:22:21] jdlrobson: thanks. it looked like a non issue. but no assuming permitted. [23:22:38] dr0ptp4kt: i'll create a bug for Zero [23:23:01] jdlrobson: you mean a fresh task for the unstructured sprint? [23:23:08] er, semi-structured sprint [23:23:08] yup [23:23:12] jdlrobson: cool [23:23:12] i'm on it [23:24:09] jdlrobson, dr0ptp4kt: if you mean a bug for QUnit tests, I already did https://phabricator.wikimedia.org/T129421 [23:24:43] dr0ptp4kt: we may even want to consider merging Zero's code into MobileFrontend [23:24:53] jdlrobson: [23:25:44] dr0ptp4kt: at least the frontend code :/ [23:26:11] eek window.history.replaceState('', document.title, window.location.pathname) [23:26:14] jdlrobson: yeah, worth consideration. in a more ideal world it would operate such that it could work for rl-capapble UAs on desktop, too (i.e., a strictly javascript treatment there)...and continue supporting all classes on mobile. but no silver bullets. [23:26:33] it's using some very old code. [23:26:51] it seems to mostly hide things? [23:26:54] For T104094, which UI action should I associate to setOnLongClickListener? Notice the ticket compares an options menu to an ImageView.. they do not have the same built-in "tooltip" behaviour. [23:26:54] T104094: No long press hints in search mode - https://phabricator.wikimedia.org/T104094 [23:32:15] jhobs: are you gonna SWAT that fix today or is yurik on it? [23:32:41] jhobs: okay so now i'lll ask again :) anyupdates on https://phabricator.wikimedia.org/T123549 ? [23:32:42] jdlrobson, its better if jhobs could add it to swat - he has it set up for testing [23:33:14] jhobs, can you test it later? [23:33:16] jdlrobson: working on it now; see earlier message [23:33:30] i remember you had to go, is that still the case? [23:33:46] yurik: yeah. If you can be around for the deployment and ping me I can test it real fast after it's live [23:33:59] since my IP's already on Zero:TEST [23:34:01] jhobs, sure,i'll be around [23:34:10] ok cool, I'll schedule it with your name [23:34:18] i can't really add my own ip because i think its shared with half the city :) [23:34:23] no worries [23:34:28] thx [23:34:33] yurik: just ping me on this channel when it's live [23:34:38] ok [23:38:23] jhobs: would pairing help or have you got it worked out? [23:42:13] niedzielski: Unless you have anything against it, I will try to position a Toast strategically next to the ImageView upon detecting a long press. I can't adopt the exact same behaviour as highlighted in the Task as the View != option menu [23:43:08] maroloccio: hey! does setting android:contentDescription in the XML for the button not work? [23:43:51] jdlrobson: should probably be fine but I'll ping you if I don't get it quickly [23:44:00] k [23:44:33] android:contentDescription is already set, but that is mostly intended for screen readers and visually impaired users? [23:47:01] niedzielski: Alternatively, have you found a reliable way to use android:contentDescription as a container for a string to be displayed in a "hovering tooltip" associated with long presses? Does not seem to be a default as it is on an ActionBar item, for instance [23:54:56] maroloccio: sorry missed your message. hm, i thought setting the contentDescription gave hint text but if already set, i guess not :/ let me take a peek [23:59:32] maroloccio: hm, so i'm looking view_article_menu_bar.xml and i see it has some ImageButtons in it. these buttons use contentDescription and the hover text works fine. i wonder if just change the trashcan from an ImageView to an ImageButton will fix things? [23:59:50] niedzielski: I can develop a minimalistic example of how a "Toast" would work, based on setGravity, {x,y}-offsets and .getLocationOnScreen()