1693168 - Port bug 1685313: Display chrome window modal dialogs from the prompt service inside windows (use prompts.windowPromptSubDialog)

Status: NEW

(Thunderbird :: Upstream Synchronization, task)

Comment 1

[Richard Marti (:Paenglab)]

Attachment: 1693168-modal-dialog-in-window.patch

This is my try to make the modal dialogs in-window. With this we should no more have issues with dialogs that open on the wrong screen.

When I try to open the folder properties dialog it is still opened as a normal dialog.

Alex, Magnus, is this approach wrong or am I missing something? Is still code missing FX uses to open the new print dialog in-content? Maybe you can help me making this work?

Comment 2

[Alessandro Castellani [:aleca]]

Comment on attachment 9203522 [details] [diff] [review]
1693168-modal-dialog-in-window.patch

Review of attachment 9203522 [details] [diff] [review]:
-----------------------------------------------------------------

I think porting this makes sense for a lot of our dialogs, but only for those that don't require the user to look at the main window for a visual feedback.

For example, the folder properties shouldn't pass through this because of the custom color option.
If we dim the main window, the user won't have an accurate representation of the color picked in the folder.
This issue already happens in macOS BigSur, where dialogs are all forced as modals with the main window blurring.

Side note: I think the folder properties should be reworked in a tab or in the content due to the amount of content listed there.

Comment 3

[Magnus Melin [:mkmelin]]

Comment on attachment 9203522 [details] [diff] [review]
1693168-modal-dialog-in-window.patch

Review of attachment 9203522 [details] [diff] [review]:
-----------------------------------------------------------------

With the below it kind of works. E.g. trying to create a folder with the same name as an existing one, I get the inline-alert, But behind the other dialog. 
In general seems like something we want, but with dialog in dialog setup, I'm not sure how easy it is to make it all work properly.

::: mail/base/content/msgMail3PaneWindow.js
@@ +557,5 @@
>      item.hidden = hideItems;
>    }
>  }
>  
> +// Handle window-modal prompts in-window.

Unclear which file(s) we should put this in, as it will be needed outside the 3pane as well. A new one, or mailWindow.js perhaps...
Or then this is fine for now since a browser is needed and that may not be available everywhere :/

@@ +593,5 @@
> +  _open(uri, args) {
> +    // Get this offset before we touch style below, as touching style seems
> +    // to reset the cached layout bounds.
> +    let offset = window.windowUtils.getBoundsWithoutFlushing(
> +      gBrowser.selectedBrowser

We don't have a gBrowser.

Replace with `document.getElementById("tabmail").selectedBrowser` to get it working

Comment 4

[Magnus Melin [:mkmelin]]

Regarding the folder properties, that does seem like something which is traditionally in a dialog.
For the folder color changes, probably better to just render a folder icon inside the dialog instead so that the user can check it there. Having to check outside/behind a dialog seems a bit odd (and apparently BigSur wants to practically enforce it).

Comment 5

[Richard Marti (:Paenglab)]

• let offset = window.windowUtils.getBoundsWithoutFlushing(

•  gBrowser.selectedBrowser
  

We don't have a gBrowser.

Replace with document.getElementById("tabmail").selectedBrowser to get it
working

I tried this and I see no change. It doesn't work for me. Maybe you see the Ubuntu behaviour of centring modal dialogs in the window and dimming a bit the window content?

Comment 6

[Magnus Melin [:mkmelin]]

No, I mean, it works for the alert that pops up in the background, which I suppose would be expected. This is what it looks like: https://pasteboard.co/JQ1DZug.png. Without the patch I get the normal alert() dialog

Comment 7

[Magnus Melin [:mkmelin]]

prompts.windowPromptSubDialog could (possibly) be affect something here.

Comment 8

[Takanori MATSUURA]

Upstream bug 1685313: has been fixed.