Page MenuHomePhabricator

Edits to wish description not being saved correctly
Open, HighPublicBUG REPORT

Description

What is the problem?

I have experienced a few cases while editing Describe your wish in the wish submission form where my edits have not been saved correctly.

This has happened for both the Source and Visual Editors.

Examples:

  • In the Source Editor, adding a template via the editing toolbar saves nothing but newlines.
  • With the cursor in the VisualEditor, if you click "Publish changes" (without first clicking outside of VE) any edits you made are not recorded (either it is a null edit or it might record a few newlines).

I haven't had much time to investigate this. It is possible that there are even more cases where this happens.

Steps to reproduce problem

Source Editor:

  1. Edit an existing wish with the wish submission form
  2. In Describe your wish, open the + menu in the editor toolbar and select "Template"
  3. Insert a template (for example, you could use the Colon template)
  4. Click "Publish changes"

Expected behaviour: The template you inserted appears in the wish.
Observed behaviour: No edit is recorded (no new change in the page history) or an edit that is just newlines.

VisualEditor:

  1. Edit an existing wish with the wish submission form
  2. The "Publish changes" button may be disabled. If so, enable it by clicking in the Describe your wish field and then outside again.
  3. Switch to VisualEditor
  4. Make some edits
  5. Without clicking on any other part of the form, click "Publish changes"

Expected behaviour: The changes you made appear in the wish.
Observed behaviour: No edit is recorded (no new change in the page history) or an edit that is just newlines.

Environment

Browser: Firefox 115
Wiki(s): https://wishlist-test.toolforge.org

Screenshots

Inserting a template with source editor:

inserting_template.png (577×1 px, 29 KB)

The diff shows only a newline:

inserting_template_diff.png (516×991 px, 54 KB)

Details

TitleReferenceAuthorSource BranchDest Branch
Disable form fields on submission; log errors to consolerepos/commtech/wishlist-intake!141musikanimaldisable-on-submitmain
WishlistIntake: Introduce preSubmitPromises to run things before submitrepos/commtech/wishlist-intake!129musikanimalpre-submit-promisesmain
Customize query in GitLab

Event Timeline

Insert a template (for example, you could use the Colon template)

The not-so-fun part of fixing this bug -- accidentally clicking on "Insert image" instead of "Insert template" and typing the suggested! :-P


VisualEditor:

  1. Make some edits [to the description field]
  2. Without clicking on any other part of the form, click "Publish changes"

The above MR fixes this race condition.

Source Editor:

  1. In Describe your wish, open the + menu in the editor toolbar and select "Template"
  2. Insert a template (for example, you could use the Colon template)
  3. Click "Publish changes"

This is apparently a separate issue. Following the repro steps, I see the VE data model does not contain the "Colon" template. I also see the wikitext is getting "converted" after insertion, which shouldn't happen because it's already wikitext. I think that might have something to do with this bug. We may want to move this to a separate task.

Also, a follow-up to this task might be that after clicking Submit, to disable all form elements as well the submit button. There are actually several things happening behind the scenes, so we need a loading state. Disabling fields is the normal convention we often see on the web, i.e. such as submitting this comment here in Phabricator. Does that sound okay @JSengupta-WMF? Related: T367917

samwilson merged https://gitlab.wikimedia.org/repos/commtech/wishlist-intake/-/merge_requests/129

WishlistIntake: Introduce preSubmitPromises to run things before submit

@MusikAnimal yes disabling the fields and the button while the form is submitting is fine. Do we have an existing loading state or do you need a design for that?

Okay, thanks! We can just disable the form fields and see what you think. It's all Codex so yes, there are disabled states already.