Page MenuHomePhabricator
Create Task
Maniphest T185944

OOUI window ready process runs before window is actually ready (Category drop down list gets sticky when it's open while closing the dialog)
Closed, ResolvedPublic
Actions

Description

Steps to reproduce:

  1. Go to Page Settings>Categories
  2. Type something to search for a category
  3. When the drop down appears, close the dialog
  4. Open the dialog again

Observed Result:
The drop down is still open in a displaced manner.

Screen Shot 2018-01-29 at 1.06.45 PM.png (533×1 px, 189 KB)

Details

SubjectRepoBranchLines +/-
ve.ui.MWMediaDialog: Fix confusion between #getSetupProcess and #getReadyProcessmediawiki/extensions/VisualEditorwmf/1.32.0-wmf.13+16 -11
ve.ui.MWMediaDialog: Fix confusion between #getSetupProcess and #getReadyProcessmediawiki/extensions/VisualEditormaster+16 -11
Update VE core submodule to master (8d0ab0587)mediawiki/extensions/VisualEditormaster+10 -6
Fix confusion between #getSetupProcess and #getReadyProcessmediawiki/extensions/VisualEditormaster+34 -39
Fix confusion between #getSetupProcess and #getReadyProcessVisualEditor/VisualEditormaster+11 -12
Ensure window ready process runs after window is made visibleoojs/uimaster+41 -43
Customize query in gerrit

Related Objects
Search...

Event Timeline

Ryasmeen created this task.Jan 29 2018, 9:14 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 29 2018, 9:14 PM
matmarex claimed this task.Jun 21 2018, 7:50 PM
matmarex edited projects, added VisualEditor (Current work), OOUI; removed VisualEditor.
matmarex subscribed.

This superficially looks similar to T155976, but I think it has a different root cause. Looks like there is a bug in OOUI where it won't wait for a window to be fully open before focussing fields inside of it :(

gerritbot subscribed.Jun 21 2018, 8:44 PM

Change 441482 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[oojs/ui@master] Ensure window ready process runs after window is made visible

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

gerritbot added a project: Patch-For-Review.Jun 21 2018, 8:44 PM
matmarex added a comment.Jun 21 2018, 8:46 PM

@Ryasmeen It seems that this has always been broken, but no one noticed before you :)

One important takeaway from the refactor: If the ready process adds some content to the window, this will now be visibly delayed (by 250 ms). Code doing that should be in the setup process. I see a few places in VE where we obviously do that wrong, but there may be others (or in other code), so this is something worth QA-ing after the OOUI change is deployed.

gerritbot added a comment.Jun 21 2018, 9:08 PM

Change 441489 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[VisualEditor/VisualEditor@master] Fix confusion between #getSetupProcess and #getReadyProcess

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

gerritbot added a comment.Jun 21 2018, 9:08 PM

Change 441490 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] Fix confusion between #getSetupProcess and #getReadyProcess

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

Jdforrester-WMF moved this task from Incoming to In progress on the VisualEditor (Current work) board.Jun 21 2018, 10:55 PM
matmarex moved this task from In progress to Code review on the VisualEditor (Current work) board.Jun 21 2018, 11:48 PM
Krinkle awarded a token.Jun 24 2018, 1:03 AM
Volker_E moved this task from Backlog to Doing on the OOUI board.Jun 27 2018, 7:59 AM
matmarex triaged this task as Medium priority.Jul 5 2018, 4:16 PM
gerritbot added a comment.Jul 10 2018, 3:40 PM

Change 441482 merged by jenkins-bot:
[oojs/ui@master] Ensure window ready process runs after window is made visible

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

matmarex added a comment.Jul 11 2018, 9:49 PM

@Ryasmeen The OOUI refactoring has also revealed a few other minor issues in VisualEditor, which are now visible on https://en.wikipedia.beta.wmflabs.org/:

  • Page options → Templates used (and other options, except the first one) opens the first panel first, then visibly flashes to the right panel
  • Insert → Template opens as empty dialog first, then the form flashes in
  • Welcome dialog first opens with three buttons, then some of them disappear (depending on whether page is visually-editable)

Quick recording for reference: https://drive.google.com/file/d/1mJhmKQOe8eEblFmQhnsNqB57tsEOdAyJ/view?usp=sharing (too large for Phabricator)

All of these are fixed by my patches above (pending review).

Similar issues might appear in other code using OOUI dialogs. I think it's unnecessary to try to check them all (it's a lot of code, and the issues, if any, are just minor visual glitches like these).

However, if QA or anyone else spots a problem like this (contents or buttons of a dialog somehow rearranging themselves immediately after it opens), please know that this is the root cause, and the issue is fixed by moving initialization code from #getReadyProcess to #getSetupProcess. #getReadyProcess should basically only contain code that focusses/activates something in the dialog. I'm not sure if we should try to announce this somewhere.

gerritbot added a comment.Jul 15 2018, 12:09 PM

Change 441489 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Fix confusion between #getSetupProcess and #getReadyProcess

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

gerritbot added a comment.Jul 16 2018, 11:16 PM

Change 441490 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Fix confusion between #getSetupProcess and #getReadyProcess

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

ReleaseTaggerBot added a project: MW-1.32-notes (WMF-deploy-2018-07-17 (1.32.0-wmf.13)).Jul 17 2018, 12:00 AM
gerritbot added a comment.Jul 17 2018, 5:02 AM

Change 446218 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (8d0ab0587)

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

Ryasmeen added a comment.Jul 17 2018, 11:42 PM
In T185944#4417645, @matmarex wrote:

@Ryasmeen The OOUI refactoring has also revealed a few other minor issues in VisualEditor, which are now visible on https://en.wikipedia.beta.wmflabs.org/:

  • Page options → Templates used (and other options, except the first one) opens the first panel first, then visibly flashes to the right panel
  • Insert → Template opens as empty dialog first, then the form flashes in
  • Welcome dialog first opens with three buttons, then some of them disappear (depending on whether page is visually-editable)

Quick recording for reference: https://drive.google.com/file/d/1mJhmKQOe8eEblFmQhnsNqB57tsEOdAyJ/view?usp=sharing (too large for Phabricator)

All of these are fixed by my patches above (pending review).

Similar issues might appear in other code using OOUI dialogs. I think it's unnecessary to try to check them all (it's a lot of code, and the issues, if any, are just minor visual glitches like these).

However, if QA or anyone else spots a problem like this (contents or buttons of a dialog somehow rearranging themselves immediately after it opens), please know that this is the root cause, and the issue is fixed by moving initialization code from #getReadyProcess to #getSetupProcess. #getReadyProcess should basically only contain code that focusses/activates something in the dialog. I'm not sure if we should try to announce this somewhere.

@matmarex : I checked the original issue and the issues you mentioned above. They all look fixed to me. However, today I found an issue with media settings dialog T199841 which sounds like what you described as buttons of a dialog rearranging themselves immediately after it opens. Not sure if it is relevant? Didn't observe this issue in any other dialog though.

matmarex added a comment.Jul 18 2018, 12:46 AM

@Ryasmeen Thanks, that is caused by this change as well! I did not notice the problem in that dialog.

gerritbot added a comment.Jul 18 2018, 12:46 AM

Change 446516 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] ve.ui.MWMediaDialog: Fix confusion between #getSetupProcess and #getReadyProcess

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

gerritbot added a comment.Jul 18 2018, 1:14 AM

Change 446218 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (8d0ab0587)

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

Jdforrester-WMF edited projects, added MW-1.32-notes (WMF-deploy-2018-07-24 (1.32.0-wmf.14)); removed MW-1.32-notes (WMF-deploy-2018-07-17 (1.32.0-wmf.13)), Patch-For-Review.Jul 18 2018, 4:31 AM
Jdforrester-WMF moved this task from Code review to QA on the VisualEditor (Current work) board.
gerritbot added a comment.Jul 18 2018, 4:51 AM

Change 446516 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] ve.ui.MWMediaDialog: Fix confusion between #getSetupProcess and #getReadyProcess

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

gerritbot added a comment.Jul 18 2018, 6:01 AM

Change 446529 had a related patch set uploaded (by Jforrester; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@wmf/1.32.0-wmf.13] ve.ui.MWMediaDialog: Fix confusion between #getSetupProcess and #getReadyProcess

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

gerritbot added a project: Patch-For-Review.Jul 18 2018, 6:01 AM
Ryasmeen added a project: Verified.Jul 18 2018, 8:44 PM
Ryasmeen moved this task from QA to Product owner review on the VisualEditor (Current work) board.Jul 18 2018, 8:47 PM
matmarex renamed this task from Category drop down list gets sticky when it's open while closing the dialog to OOUI window ready process runs before window is actually ready (Category drop down list gets sticky when it's open while closing the dialog).Jul 18 2018, 10:52 PM
matmarex mentioned this in T199838: [Regression wmf.13] Input field getting both red and blue highlight at the same time in Citation dialog.Jul 18 2018, 11:00 PM
gerritbot added a comment.Jul 18 2018, 11:26 PM

Change 446529 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@wmf/1.32.0-wmf.13] ve.ui.MWMediaDialog: Fix confusion between #getSetupProcess and #getReadyProcess

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

Stashbot subscribed.Jul 18 2018, 11:33 PM

Mentioned in SAL (#wikimedia-operations) [2018-07-18T23:33:02Z] <thcipriani@deploy1001> Synchronized php-1.32.0-wmf.13/extensions/VisualEditor/modules/ve-mw/ui/dialogs/ve.ui.MWMediaDialog.js: SWAT: [[gerrit:446529|ve.ui.MWMediaDialog: Fix confusion between #getSetupProcess and #getReadyProcess]] T185944 T199841 (duration: 00m 56s)

Stashbot mentioned this in T199841: [Regression wmf.13] All kinds of buttons from different panels of Media Settings dialog are appearing at the bottom of the dialog for a split second while editing an image .Jul 18 2018, 11:33 PM
matmarex added parent tasks: T199838: [Regression wmf.13] Input field getting both red and blue highlight at the same time in Citation dialog, T199841: [Regression wmf.13] All kinds of buttons from different panels of Media Settings dialog are appearing at the bottom of the dialog for a split second while editing an image .Jul 18 2018, 11:47 PM
Jdforrester-WMF edited projects, added MW-1.27-release (WMF-deploy-2016-02-09_(1.27.0-wmf.13)); removed Patch-For-Review, MW-1.32-notes (WMF-deploy-2018-07-24 (1.32.0-wmf.14)).Jul 25 2018, 6:39 PM
• Deskana closed this task as Resolved.Jul 30 2018, 3:26 PM
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptJul 30 2018, 3:26 PM
Addshore mentioned this in T201901: Selenium test Lexeme:Undoing removes grammatical features fails.Aug 14 2018, 12:07 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL