Page MenuHomePhabricator
Create Task
Maniphest T315636

Opening an OOUI dialog from within a dialog deadlocks the entire screen
Closed, ResolvedPublic2 Estimated Story PointsBUG REPORT
Actions

Description

Steps to reproduce:

  • Start VisualEditor.
  • Add or edit a simple Maps (Kartographer) <mapframe> element.
  • Go to the options tab.
  • Click the ••• button next to the language code input. A popup (within the maps popup) to select a language opens.
  • Now the entire screen is blocked.

It appears like this is the same issue as in T313690: Dialog buttons not working after required template param warning (due to 'inert' attribute not being cleared). I can find a whole bunch of inert attributes in the HTML. When I manually remove them all it works again.

I wonder how many other places in the hundreds of codebases we maintain have the same problem? Should we revert the patch that added these inert just to be sure? Marking this as a OOUI problem because of this.

Details

SubjectRepoBranchLines +/-
Update VE core submodule to master (8ff9a5b44)mediawiki/extensions/VisualEditormaster+1 -1
ve.ui.LanguageInputWidget: Fix dialog becoming inertVisualEditor/VisualEditormaster+1 -3
Customize query in gerrit

Related Objects

Event Timeline

thiemowmde created this task.Aug 19 2022, 6:58 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 19 2022, 6:58 AM
thiemowmde triaged this task as High priority.Aug 19 2022, 6:59 AM
thiemowmde moved this task from Incoming to Priority backlog on the WMDE-TechWish-Maintenance board.
gerritbot added a comment.Aug 19 2022, 7:07 AM

Change 824678 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[VisualEditor/VisualEditor@master] ve.ui.LanguageInputWidget: Fix dialog becoming inert

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

gerritbot added a project: Patch-For-Review.Aug 19 2022, 7:07 AM
thiemowmde moved this task from Sprint Backlog to Tech Review on the WMDE-TechWish-Sprint-2022-08-17 board.Aug 19 2022, 7:08 AM
thiemowmde claimed this task.Aug 19 2022, 7:13 AM
thiemowmde set the point value for this task to 2.
thiemowmde added projects: Editing-team, Unplanned-Sprint-Work.

I found the same code as in https://gerrit.wikimedia.org/r/816798 and applied an identical change to the LanguageInputWidget. Unfortunately this one sits in the ve lib where our team can't +2 patches.

Luckily it looks like these are really the only two places in all code we have that do this: https://codesearch.wmcloud.org/search/?q=new%20ve.ui.WindowManager&files=%5C.js%24

gerritbot added a comment.Aug 19 2022, 3:32 PM

Change 824678 merged by jenkins-bot:

[VisualEditor/VisualEditor@master] ve.ui.LanguageInputWidget: Fix dialog becoming inert

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

gerritbot added a comment.Aug 19 2022, 3:42 PM

Change 824761 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (8ff9a5b44)

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

gerritbot added a comment.Aug 19 2022, 4:00 PM

Change 824761 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (8ff9a5b44)

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

matmarex added a comment.Aug 19 2022, 4:02 PM

Thanks @thiemowmde.

In T315636#8168083, @thiemowmde wrote:

Luckily it looks like these are really the only two places in all code we have that do this: https://codesearch.wmcloud.org/search/?q=new%20ve.ui.WindowManager&files=%5C.js%24

I think there might be more, ve.ui.WindowManager is a subclass of OO.ui.WindowManager, which is used more widely and which (I think) would be affected by the same issue:

https://codesearch.wmcloud.org/search/?q=new%20OO.ui.WindowManager&files=%5C.js%24

At a glance, most uses immediately append to body, and would be fine. GrowthExperiments does some weird stuff, but I tested in production and their weird stuff works (at least in the one case I tried). But I haven't audited them all carefully…

Maintenance_bot removed a project: Patch-For-Review.Aug 19 2022, 4:30 PM
ReleaseTaggerBot added a project: MW-1.39-notes (1.39.0-wmf.26; 2022-08-22).Aug 19 2022, 5:00 PM
thiemowmde moved this task from Tech Review to Demo on the WMDE-TechWish-Sprint-2022-08-17 board.Aug 22 2022, 8:45 AM
thiemowmde added a comment.Aug 22 2022, 10:23 AM

I'm afraid I can't be of much help here. But maybe it's worth having a look at these as well: https://codesearch.wmcloud.org/search/?q=body%5CW*append.*overlay&files=%5C.js%24

VPuffetMichel moved this task from To Triage to Triaged on the VisualEditor board.Aug 25 2022, 3:48 PM
awight updated the task description. (Show Details)Aug 26 2022, 7:11 AM
thiemowmde moved this task from Demo to Done on the WMDE-TechWish-Sprint-2022-08-17 board.Aug 30 2022, 8:39 AM
WMDE-Fisch closed this task as Resolved.Aug 30 2022, 8:39 AM
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