Disclaimer: I work for or provide services to the Wikimedia Foundation. However, the Foundation does not vet all my activity, so edits, statements, or other contributions made by this account may not reflect the views of the Foundation.
Thu, Apr 8
@alexhollender I don't think there's anything that Editing or general template-behavior has to say about this. From our end we just wind up creating wikitext tables. The Parsing team could change the output generated from wikitext tables, but there's enough complex possible outputs from table markup that I'd be concerned about breaking edge cases or community templates.
Interestingly, I think that 2 is the most-complex one to implement, because those links are already meddled with by VE so we'll need to be careful to not trip over each other.
Otherwise we get a bunch of dirty diffs.
Unless I'm misunderstanding the goal or your objection, we wouldn't be getting dirty diffs because we'd not be committing the output of the build process.
Wed, Mar 31
With https://gerrit.wikimedia.org/r/c/mediawiki/extensions/SecurePoll/+/676086 having been merged, I think we can go one further and say that getQuestionForm can be documented to return a OOUI\FieldsetLayout.
Tue, Mar 30
Tue, Mar 23
This one's not really QA-able until the tally job patch is done.
Sat, Mar 20
Thu, Mar 18
For what it's worth, you shouldn't need to do any direct work to add a button. There's already code for having a standardized delete button on context items, it's just restricted to the MobileContext. If you look up ve.ui.MobileContext.static.showDeleteButton you can trace through how its display is triggered.
Tue, Mar 16
I tried enabling the script, and when hovering over an IP address on RecentChanges I just got this error: Uncaught (in promise) TypeError: Failed to fetch. This turned out to be because my adblocker dislikes ipinfo.io. Once I disabled that temporarily, it worked -- though I note that none of the ipinfo data is actually being used in the RecentChanges popup as far as I can see?
Mon, Mar 15
Because the patch update just now actively changed the layout...
This was deliberately implemented by T271327.
Sun, Mar 14
Back to code review until someone approves the patch, I guess.
Mar 12 2021
@Prtksxna bumping this back into design review so you can say whether you're okay with this.
Mar 11 2021
That patch is option 3, above. It winds up being really-equivalent to the current design, just with the OOUI radio widgets in the table.
Mar 10 2021
In the interest of seeing how it works, I wound up using wmf-utils for a recent helper script for some docker work: https://gerrit.wikimedia.org/r/c/wmf-utils/+/670320
Mar 9 2021
Well, the error did disappear before that, and I can't find any for VisualEditorFeatureUse on that new dashboard, so presumably my conclusion holds up...
Checking the logstash search, these errors appear to have stopped as of March 4th -- i.e. as of the last train deploy to group2.
I went with leaving the + sign, mostly because I think it leads to more clarity now that the inputs can wrap lines.
It'll be a 5 minute patch to fix that, so up to you if you'd like the administrative overhead of a new ticket.
Mar 8 2021
I'll go ahead and do the more aggressive switch to getInputOOUI in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/SecurePoll/+/669715/ since I think you'd want to do it here anyways.
Mar 3 2021
- pick a vote threshold
- above said threshold show a warning, but still have the button available
Mar 1 2021
I think this one can skip QA -- it doesn't really have anything specific to test.
Feb 26 2021
I can do all the other page-ports quite quickly, but I want to get the review on the service done first so I don't wind up having to change and rebase everything if any major changes are requested.
Feb 25 2021
Presumably, the complaint is about this diff?
I think it's more-specifically about this diff -- the other one just shows the undoing of the specific error.
Feb 24 2021
You probably already thought about this (sorry) but you could also run a check against the centralauth database and filter out locked accounts (i.e. where gu_locked = 1), which in theory should find every staff account and could be done before an election by someone with access.
Do we lock (WMF) staff accounts when staff leave the foundation, such that this query+check would maybe get us "current staff"... or is that something which we can't work out from the database alone?
Megan: in the context of EditAttemptStep does page include edit attempts initiated via section edit links? Schema:EditAttemptStep leads me to think "yes," tho I wanted to be sure.
It's the integration, so yes. (You could distinguish section ones from init_type. But only for source mode, because we still haven't released visual section editing anywhere.)
Feb 23 2021
I should note that I couldn't actually reproduce triggering a VisualEditorFeatureUse event which didn't have a session id, so this being the cause is largely speculation. Still, absent some really weird case I think this should fix any possible route to such an event being generated.
Mobile is the most likely situation for us not getting that final abort if they abandon the tab -- if they leave the browser app-or-tab, it can get pruned for memory conservation without ever getting to run events again. (But we should still get it if they're e.g. reloading the page.)
I think we should try to piece together complete sessions from these events, and see what events (if any) happen after the "unpaired" open events.
Getting out without a dialog-whatever event requires that the dialog be closed in such a way that the getTeardownProcess method is never called on it.
There does seem to be a useful pattern: "integration": "discussiontools" was present on every event I checked. I suspect that this means there's a path to a VisualEditorFeatureUse event firing before the first EditAttemptStep init event, which is normally what would generate the sessionid.
If one doesn't exist yet (and I think it doesn't), we could suggest that a user group be made for staff? It looks like the eligibility list is supposed to support "Include users in these groups, regardless of edits or other groups", and so that'd cover the whole thing without encoding any specific business logic. (It looks sort of broken because of hidden fields and cloner at the moment, so I'm kinda guessing about how it's supposed to work.)
Feb 22 2021
Outside of the technical implications, I would also like to run this by Legal to make sure we aren't doing something we are not supposed to do.
Feb 19 2021
I would expect there to be far fewer number of non-discussion tool edit attempts in the test group compared to the control group (especially on talk pages) as they are shown the reply tool as default. @DLynch - Any ideas as to why this might be?
Feb 18 2021
(Brought up on our meeting, and mentioning here for completeness.)
@Tchanders I'm fine with that, and will update the patch.
This only occurs if the "more than 10 wikis are known about" case is hit, and is because in an autocompleteselect widget is used. This is one of the rare widgets which never got ported to OOUI exactly as-is, probably because it's not used anywhere in core. Codesearch turns up precisely one other use of it anywhere, in the ViewProtect extension where it's used only as a fallback if the combobox type doesn't exist.
Feb 17 2021
An updated stack trace, since the line numbers have shifted since the one in the ticket:
Feb 15 2021
For what it's worth, currently if you open the "add topic" link in a new tab, you'll get the old way.
Feb 13 2021
Feb 12 2021
Example: someone could make a change to the document without them being able to undo or redo that change because no record exists of that change being made.
Worse, in that scenario, attempting to undo/redo would break the document in unpredictable ways, because the transactions applied wouldn't account for the changed data.
Feb 11 2021
That patch changes the current state of affairs: the existing config setting 5 wikis to sample at 0.2 is removed, and replaced by oversampling all logging from DiscussionTools.
@MNeisler Actually, question, for data-integrity purposes do you (a) actually use EventLogging's oversampled attribute, and (b) if so should we be logging things as oversampled if they're being disproportionately sampled between tools? (Since it presumably gets more complicated to analyze comparisons if the rates are different?)
Events on all wikis where the New Discussion Tool and/or Reply Tool are available should be oversampnled.
Feb 9 2021
Work on this has been happening in T264885.
Work on this has been happening in T264885.
Feb 8 2021
Explanation of sampling rate quirks:
In a backport window we would need to put:
Feb 3 2021
Just as a note, we don't currently have a convenient setting for oversampling the logging for just one part of DiscussionTool. It's all-features or nothing, at the moment.
but yeah it would be nice to make this easier and more up front for people.
I'm somewhat motivated by my team affiliation to argue for the configurations where it's possible to use VisualEditor being the defaults, admittedly. :D
Having an easy and somewhat-official way to get a mediawiki+mysql instance running is what's needed. Whether it's the default or not doesn't really matter, I think, so long as we're fairly up-front about what the trade-offs are. (E.g. I think that DEVELOPERS.md should have an "only choose sqlite if..." disclaimer.)
Feb 2 2021
It seems that a non-sqlite image is a requirement if you want to do development that touches on Parsoid via Restbase (so, VisualEditor / DiscussionTools stuff mostly), because of clashing database locks.
It does sound, from reading docker docs, that Mac and Linux behave differently for this, so it's possible that could be why our experience is different. I can say for me that I was never trying to use port 80, so there's at least one route into the issue that doesn't depend on that.
Based on my cargo-cult usage of docker, I think that URL should be http://host.docker.internal/ rather than localhost.
Just ran into part of this and did some work on debugging it for myself. The VE one, at least, is down to how Docker handles connecting internally -- localhost doesn't work, and you have to use host.docker.internal instead. Therefore I had to do this in my LocalSettings.php:
Plausibly related to whatever was going on in T273006.