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.
Sat, Aug 11
Given the short-term prototyping goals of this, I'd assume that we'd want to quickly spin an instance up and discard it on a fairly short timeframe. Assuming that we can properly automate all that content-importing and patch-applying, this'd get us to a low maintenance state. Given that Vagrant itself is doing fine, of course.
Fri, Aug 10
I suppose that I'm not opposed to the combined field-button per-se, I'm just with matmarex in not really liking it in this specific instance. I'd probably be most visually happy just restoring the separation in this case. (And maybe other similar ones with this aesthetic incompatibility.)
Thu, Aug 9
I suspect the most relevant bit for that is ve.init.mw.Target.prototype.setupSurface, since that's where we actually turn the DOM into our model. (Though I grant I haven't stepped through and verified that the <style> hasn't been quietly stripped somewhere else where I didn't see it...)
Yeah, seems like that has indeed helped.
The requirement is more specific than just switching to VE -- it has to be switching to VE from the classic editpage, rather than from NWE, because that causes a load of VE without the underlying view of the page existing.
Additional criteria: ease of deploying an arbitrary patch to the instance.
Wed, Aug 8
(Note that there is no data loss, for VE or other affected tools, just a misleading message.)
The incompleteForm message part will be fixed when my patch on T199554 is merged. I'm not sure if that's the sole part that's holding up going to the conflict resolution interface, though -- I'll double-check it.
@Esanders: Makes sense, and I agree on the general case. If you merge it, I can make a new task for fixing it up more permanently.
Tue, Aug 7
Question 1 is what I was asking above in my first comment on this task, so I'd say that's somewhat-undecided. I think 2 is unintended, though; it should switch to cell selection first.
It's the same as how pasting * doesn't make it become a list item. Which maybe it should? Not sure.
@cscott: Oh, I totally agree that this UX isn't optimal, and that we should add it to @iamjessklein's big pile of VE design thoughts. I'm just happy to fix the power-user case right now while we wait on that. (And we'll presumably need whatever functionality we add via resource for the nicely-designed option anyway, so that shouldn't hurt.)
Mon, Aug 6
@cscott: Nope, this is all we get from Parsoid: [[Media:Logo.png]] => <p id="mwAQ"><a rel="mw:MediaLink" href="//upload.wikimedia.org/wikipedia/commons/c/c9/Logo.png" title="Logo.png" id="mwAg">Media:Logo.png</a></p> (with the link text obviously not being reliable, because people can change that).
Absent Parsoid output/input changes, I will make those editing experiences equivalent by:
From a UX standpoint, I'd expect that "link to file page on commons" vs "link directly to media download" is a checkbox in the link options somehow. I think we'd need some insight from a designer on how to best express this w/in the current VE framework. The magical checkbox would determine whether VE gave us back the mw:MediaLink or mw:ExtLink type.
Okay, so. I've gone and read wt2html/tt/LinkHandler.js and html2wt/LinkHandler.js in Parsoid, and I think that this never worked in VE, since Parsoid added this MediaLink stuff back in May of 2017.
Alas, testing indicates the behavior existed prior to that. So I'll just fix it rather than digging.
If I'm speculating wildly, https://gerrit.wikimedia.org/r/c/mediawiki/extensions/VisualEditor/+/436544 did change a bunch of our parsoid-and-links interactions recently.
We have no explicit handling of mw:MediaLink, so I think what's happening here is just that it's treating it like any other link and falling back on that generic behavior.
Sat, Aug 4
There's some discussion of this in T124305 as well.
Thu, Aug 2
Not firefox specific. It does require that you have both multiple edit tabs, not be using NWE, and trigger the switch by clicking the "edit source" tab rather than through VE's menus, though, which I suspect is why Dan couldn't reproduce it.
I think this is a dupe of T136267.
Wed, Aug 1
Tue, Jul 31
@Deskana: "active node" means one which can have cursor focus within it, in a way that's somewhat separate from the general article flow. This patch makes it so that moving your cursor with the arrow keys doesn't keep it trapped within those nodes. E.g. if you're in a table caption and press the up arrow, you would previously have stayed in the caption, but now you'll find the cursor in the preceding content.
Am I right in thinking that T199087 is a duplicate of this? Because I have a patch for that which gets us:
For the table caption case, should we special case it so that it jumps the selection into the table rather than skipping it?
Actually, new development, T200525 made it so that arrows will move out of it.
Mon, Jul 30
As of T192163 this is fixed, insofar as the tab key now works to move in/out of table captions.
Thu, Jul 26
Wed, Jul 25
@santhosh Context popups trigger in reaction to contextChange events, which I'm fairly sure only fire in response to things which happen to an enabled surface. Plus, complications of the surface not having a context if there's not a selection for it to know what that context should be. You could plausibly manually override this by directly asking ve.ui.contextItemFactory.getRelatedItems for context items for parts of the document you're interested in, and displaying those however you please -- that said, context items have a few parts which expect to interact with an enabled document (for links, that'd be the remove-annotation and select-label buttons), and those would cause issues with a disabled surface as well.
@Trizek-WMF: You'll find extensive discussion of that approach in T55973, if you want to see the arguments. I had a patch which worked that way as well, and it was less popular than the one which we went with in the end.
Wed, Jul 18
I can explain why things are the way they are, at least.
Tue, Jul 17
I think this is just an uncaught duplicate of T121183? (And so, fixed.)
Mon, Jul 16
Make the converter give us a CommentMetaItem instead? We try to do that for things like comments mixed into tables between rows already, I think.
Jul 12 2018
Sorry, do you mean a surplus new line is added in addition to the one you copied? The behavior you describe, as I read it, is what I'd expect from a text editor.
Jul 11 2018
Let's see... this is ve.dm.VisualDiff.prototype.flattenList failing when it finds a CommentNode that's a child of a ListNode.
Jul 9 2018
Jul 3 2018
Esanders: Looks like it was removed in 24fe991c00. ...though I can't see any evidence that the .oo-ui-actionFieldLayout-field class that's removed there would ever actually have been created by ActionFieldLayout. So.
Jul 2 2018
QA note: when merged this patch has a chance of regressing T157755, so double-check behavior with single/multi edit tabs on pages with __NEWSECTIONLINK__.
Jun 27 2018
As a version input, I tried it in Firefox 62, and also didn't see any problems.
Jun 26 2018
- Update the default message to "remove formatting" (as you say, it's better than "clear styling")
- Update the link context specifically to "remove link"
- Update the language context specifically to "remove language"
Should be today, I think, unless something blocks the train.
Note that this is totally my fault from back in 2015 with 7ecde6c05.
Jun 25 2018
Didn't strictly need to save -- any teardown would cause it. It relied on the single-edit-tab behavior of switching from veaction to action in URLs.
Jun 21 2018
We do already refer to the link text as "label" in other places -- e.g. a numbered link gets an "add label" button to swap out the "" for text -- that said, if people think it's unclear and needs explanation in this situation, now would be a good time to quickly change that before people spend effort on translations.
I do agree with matmarex's suggested approach. The issue here seems to stem from a lack of clarity (on my part, on reviewer's part, on the code's part, etc) about exactly when things are providing a mobile experience. Wiping that clean and getting a simple rule for it would make sense.
Jun 19 2018
I found the simplest way to test reproducing this was to make a template which just contains a single link, include it on an editor page, and then use dev tools to outright remove the highlight node. Leaves the link easily accessible for clicking.
Jun 18 2018
Jun 15 2018
The overlay shouldn't be loading on desktop though. Overlays only really make sense on a mobile/tablet device.
@Trizek-WMF If Ed approves and merges the patch, it could all happen today. So the timeline is pretty much "once approvals come in", unfortunately.
@Jdlrobson The merged patches should mean that the case you show there will result in the full article appearing in the editor. You can see it on beta: https://simple.wikipedia.beta.wmflabs.org/wiki/Barack_Obama?useskin=minerva&action=edit
Jun 14 2018
I've done some hunting down of this -- the issue is that the windowmanager div the toolbar (and inspectors) is in has aria-hidden=true, which makes my tab isolation code assume that nothing in it should be treated as being visible for tabbing purposes.
For what it's worth, I'm thinking of this ticket as a two-part thing. Part one is the existing patches which will restore the ?action=edit behavior of getting the full text, and part two would be working out the right place to put the UI to expose it more thoroughly. (I'd argue that the edit button at the top of a mobile page next to all the other page-level actions should trigger /all, but there's probably an argument to be had here...)
Yeh the magic password bit doesn't help this problem. mobileeditorsuppress is not exactly easy to remember and is also hard to type on mobile. I personally don't think this gains much unless it's linked to in the interface.
@Deskana: That all sounds correct.
I've updated the patch for option 1 for now. (Since it's the simple just-make-it-not-bold adjustment.)
From @Deskana I guess? (It's mobile, but it's also editing, so...)
Jun 13 2018
@Volker_E So, I feel like the styling now suggests that the action button is "inside" the input. (Maximally so in the field-with-search-icon case.) That makes it feel wrong to me when the field focus excludes the button. It breaks the "this is one unit" message that the no-spacing-shared-borders design seems to be going for.
That said, I just tried to reproduce this with the new steps, and it still doesn't produce the error for me.
It maybe looks a little weird with the focus highlight and the disabled action button. If @Volker_E would like to tweak that combination somehow?
The 400 is specifically "Unknown source".
Jun 12 2018
@Trizek-WMF it truncates itself so it fits on one line.
Jun 11 2018
I think building it as a separate context item which might not appear directly next to the link context item would be a bit confusing, just because it's explicitly scoped to act on the link annotation, whereas the pile-of-contexts is "all the overlapping annotations at this cursor location".
Okay, thanks for that! That sequence, and particularly the "xy" still being present, makes me suspect that we're failing to properly tear down the toolbar dialogs, so it's still holding on to a reference to the previous surface, resulting in the failure when we try to perform actions on a discarded surface.
It's less apparent in the screenshot there, but in the full MW-VE experience, having that separate-section styling helps to distinguish it from the preview a bit:
That patch makes it so that visiting a ?action=edit URL will load the full page's text in the overlay editor. Sort of a preserve-the-status-quo patch.
I've made T196915 for the edit-a-full-page issue.