Page MenuHomePhabricator

The "Apply Changes" button in Media Settings dialog for newly added images does not get enabled after applying/changing the link of the caption, when the cursor is at the beginning index
Closed, ResolvedPublic1 Story Points

Description

Steps to reproduce:
1.Open a page with VE
2.Insert an image with a caption
3.Now re-open the media settings dialog again
4.While the cursor is at the first index of the dialog,click on the link inspector icon and apply a link to the caption

Observed Result:
The "Apply Changes" button in the media settings dialog does not get enabled.

Details

Related Gerrit Patches:
mediawiki/extensions/VisualEditor : masterSurfaceWidgets should listen to history
VisualEditor/VisualEditor : masterCheck staged changes in surface hasBeenModified

Event Timeline

Ryasmeen created this task.Jan 5 2015, 8:18 PM
Ryasmeen raised the priority of this task from to Needs Triage.
Ryasmeen updated the task description. (Show Details)
Ryasmeen added a project: VisualEditor.
Ryasmeen added subscribers: Aklapper, Ryasmeen.
Krenair updated the task description. (Show Details)Jan 5 2015, 8:27 PM
Krenair set Security to None.
Ryasmeen renamed this task from The "Apply Changes" button in Media Settings dialog does not get enabled after applying/changing the link of the caption, when the cursor is at the beginning index to The "Apply Changes" button in Media Settings dialog for newly added images does not get enabled after applying/changing the link of the caption, when the cursor is at the beginning index.Jan 5 2015, 8:40 PM
Ryasmeen assigned this task to Catrope.

This only happens to new images, and it's extremely weird. This bug previously happened to *all* images, then it was fixed (I believe by Roan's correction to inspectors) and now it works for existing images and fails on new images.

It seems like we have a difference with the way the document is defined to new images, but the caption document saves correctly when the changes are applied.

Jdforrester-WMF triaged this task as Medium priority.Jan 6 2015, 4:39 PM
Jdforrester-WMF removed Catrope as the assignee of this task.Jan 7 2015, 2:25 AM

Change 183303 had a related patch set uploaded (by Mooeypoo):
Check staged changes in surface hasBeenModified

https://gerrit.wikimedia.org/r/183303

Patch-For-Review

This is an issue with the way the surface calculates its "hasBeenModified" state, and it happens also in citations (same bug)

We need to find a way to have the surface account for staged transactions (which mean changes from inspectors) so dialogs can enable the "Apply" button when links or language annotations are applied to text inside the surface widget.

Change 184409 had a related patch set uploaded (by Mooeypoo):
SurfaceWidgets should listen to history

https://gerrit.wikimedia.org/r/184409

Patch-For-Review

Change 183303 abandoned by Mooeypoo:
Check staged changes in surface hasBeenModified

Reason:
Fixed in ve-mw instead

https://gerrit.wikimedia.org/r/183303

Change 184409 merged by jenkins-bot:
SurfaceWidgets should listen to history

https://gerrit.wikimedia.org/r/184409

Verified the fix in Betalabs and test2

Jdforrester-WMF edited a custom field.Mar 9 2015, 6:07 PM