This is observed while trying to select image in "White tie" article on enwiki. It's much easier to reproduce when same article is loaded as source page in ContentTranslation.
Description
Description
Details
Details
Related Objects
Related Objects
- Mentioned In
- T198340: ve.dm.ElementLinearData#getRelativeOffset gives incorrect results when given offset inside a node whose children are ignored for cursoring
- Mentioned Here
- T198340: ve.dm.ElementLinearData#getRelativeOffset gives incorrect results when given offset inside a node whose children are ignored for cursoring
rGVEDfab33197b31c: Merge "LinkContextItem: add label information to the context"
Event Timeline
Comment Actions
Change 441091 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[VisualEditor/VisualEditor@master] ve.dm.Document: Handle no valid offsets in #getNearestCursorOffset
Comment Actions
I am fixing a symptom here, but maybe there is a more interesting underlying issue here. As far as I can tell, what happens here is that the browser gives us a selection starting between the <figure> and <figcaption> tags, and when trying to fix it up to a valid selection in our data model, VE explodes because neither this offset, nor any offset before it in the document, are a valid place for a selection to start. We already have code to handle this case and find an offset after instead, but it was never reached due to the exception.
Comment Actions
@dchan I did find a more interesting underlying issue.
It seems there is a bug in ve.dm.ElementLinearData#getRelativeOffset in the code which is supposed to skip over nodes "whose children should be ignored", such as figures (ve.dm.BlockImageNode). When given an invalid offset at the beginning or end of such a node (inside it), and asked to cursor outside of it, it will fail to find any valid offsets and return -1.
This limitation is actually described in the code comments: "This clearly won't work if you start inside such a node, but you shouldn't be doing that to being with". Well, we do that, ever since we allowed inline editing of figure captions (ve.dm.BlockImageCaptionNode) ;)
It should probably try the the other direction first, like it does normally, and only fail if there are no valid offsets inside at all?
The result is usually okay most of the time, since ve.dm.Document#getNearestCursorOffset also explicitly checks both directions, so as long as we fix the exception (which my patch does) the user-visible behavior will be correct. But we might want to think about this.
I added/commented some test cases in the patch, documenting the current weird behavior.
Comment Actions
Change 441091 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] ve.dm.Document: Handle no valid offsets in #getNearestCursorOffset
Comment Actions
Change 442202 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (fab33197b)
Comment Actions
Change 442202 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (fab33197b)
Comment Actions
If the immediate problem has been fixed—which it appears it has—then I'm calling this resolved. Any other or bigger issues should be filed in other tasks.