Closed Bug 1855045 Opened 2 years ago Closed 2 months ago

Implement "input.fileDialogOpened" event

Categories

(Remote Protocol :: WebDriver BiDi, enhancement, P2)

enhancement
Points:
5

Tracking

(firefox147 fixed)

RESOLVED FIXED
147 Branch
Tracking Status
firefox147 --- fixed

People

(Reporter: whimboo, Assigned: jdescottes)

References

(Blocks 3 open bugs, )

Details

(Whiteboard: [webdriver:m18][wptsync upstream][webdriver:relnote])

Attachments

(5 files, 2 obsolete files)

No description provided.
Points: --- → 5
Priority: P2 → P3
Whiteboard: [webdriver:m9] → [webdriver:backlog]
Summary: Implement "input.fileDialogIntercepted" event → Implement "input.fileDialogOpeneed" event
Summary: Implement "input.fileDialogOpeneed" event → Implement "input.fileDialogOpened" event
Whiteboard: [webdriver:backlog] → [webdriver:m12]
Whiteboard: [webdriver:m12] → [webdriver:m13]
Whiteboard: [webdriver:m13] → [webdriver:m14]
Whiteboard: [webdriver:m14] → [webdriver:m15]
Depends on: 1951457
Duplicate of this bug: 1952946
Whiteboard: [webdriver:m15] → [webdriver:m16]
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Blocks: 1954676
Pushed by nfay@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/788c98753f08 [wdspec] Temporarily disable most of the file dialog opened tests. CLOSED TREE
Assignee: hskupin → nobody
Status: ASSIGNED → NEW
Keywords: leave-open
Priority: P3 → P2
Assignee: nobody → hskupin
Status: NEW → ASSIGNED

Backed out as part of backing out other related changesets (Bug 1944565, Bug 1710425)

Backout link

Flags: needinfo?(hskupin)

The reason for the backout was from the other bugs. We can re-land this one.

Flags: needinfo?(hskupin)
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/25f1f22c50f7 [wdspec] Temporarily disable most of the file dialog opened tests. r=webdriver-reviewers,Sasha
Assignee: hskupin → nobody
Status: ASSIGNED → NEW
Whiteboard: [webdriver:m16] → [webdriver:m17]
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Attachment #9506355 - Attachment is obsolete: true
Assignee: jdescottes → nobody
Status: ASSIGNED → NEW
Whiteboard: [webdriver:m17] → [webdriver:m18]

I was looking a bit at what we need here.

First observation is that file dialogs cannot be handled with the observer notification "common-dialog-loaded" used in our PromptListener.

Since we don't support automatic prompt handling in BiDi yet (Bug 1905086) and file prompts are not in scope of browsingContext.handleUserPrompt, so we can probably leave this out of this bug.

With this bug, clients will be notified about file prompts, but will have no way to interact with it. That being said we should still try to implement the event in a way that is compatible with dismissing the file prompt later on, because that requirement will come up as soon as Bug 1905086 is addressed.

I think a reasonable technical solution here would be to reuse the same pattern as the MockFilePicker used in our tests, and register another FilePicker for WebDriver BiDi, which would allow to emit events when the file picker is requested and potentially dismiss the request, or fallback to show a real filepicker instance.

https://searchfox.org/firefox-main/rev/88121de0ccd1a6e364645b9971f36c5eb00f4317/testing/specialpowers/content/MockFilePicker.sys.mjs

Gijs, do you have any suggestion on how to monitor (and potentially interact) with File Prompt dialogs for automation (WebDriver BiDi) scenarios. Do you think reusing the MockFilePicker approach could be a good solution here?

Flags: needinfo?(gijskruitbosch+bugs)

(In reply to Julian Descottes [:jdescottes] from comment #9)

I was looking a bit at what we need here.

First observation is that file dialogs cannot be handled with the observer notification "common-dialog-loaded" used in our PromptListener.

Sort of more generally: I think on all platforms our "open file" or "save file" dialogs are all OS-implemented. That is, we hand everything off to the OS, including where the dialog is on screen, styling, etc. This is frequently an issue on Linux as various desktop environments / window managers have "interesting" ideas about what features such dialogs do/don't need and then users complain to us - but we cannot in fact help.

All this to say, we cannot straightforwardly automate interacting with the "real" dialogs (we'd need a per-OS automation layer that then talks to the OS dialogs we open via those OS APIs, rather than "just" some JS/C++ bits that talk to our own dialogs).

I think a reasonable technical solution here would be to reuse the same pattern as the MockFilePicker used in our tests, and register another FilePicker for WebDriver BiDi, which would allow to emit events when the file picker is requested and potentially dismiss the request, or fallback to show a real filepicker instance.

https://searchfox.org/firefox-main/rev/88121de0ccd1a6e364645b9971f36c5eb00f4317/testing/specialpowers/content/MockFilePicker.sys.mjs

Gijs, do you have any suggestion on how to monitor (and potentially interact) with File Prompt dialogs for automation (WebDriver BiDi) scenarios. Do you think reusing the MockFilePicker approach could be a good solution here?

This type of approach sounds reasonable to me, though the exact specifics of how it hooks into "regular" file picker use may be the troublesome bit.

Flags: needinfo?(gijskruitbosch+bugs)

Thanks for the feedback Gijs! At the moment the only interaction we have in the spec for the file picker is to either: allow it to be displayed (so do nothing) or dismiss it. In the dismiss case, it's hooked from https://html.spec.whatwg.org/#common-input-element-apis:show-the-picker,-if-applicable, which means the picker should not even be displayed. So I think it's fine if we consider the picker as a black box fully driven by the OS.

I started to implement something using the MockFilePicker approach, and while that works well for detecting FilePicker openings, our spec also expects that we provide the reference to the input element which triggered the picker. And since the MockFilePicker logic lives in the parent process, it doesn't really have access to this.

So I think I need another solution, fully handled in the content process. I'm thinking about adding an event or observer notification at https://searchfox.org/firefox-main/rev/e613f4df351a21871cfeadf7d5b4043ffad157b1/dom/html/HTMLInputElement.cpp#356

mFilePicker->Open(mFpCallback);

Ideally I would need to be able to "dismiss" opening the picker with this event, but if that's too complicated to setup, I think we could also fallback to using the event in content process to detect picker openings, and use the MockFilePicker solution in the parent process to dismiss the picker.

Olli: Does it sound fine to you to introduce an event or an observer notification before the mFilePicker->Open I linked above?

Flags: needinfo?(smaug)

FWIW, I tried a slightly different implementation, mocking the file picker in content processes. It's slightly better as I can also try to "find" the file input when I send the event, but it's very hacky and fragile, so I really think an observer notification will be the best approach here. I will try to craft a patch for this.

Assignee: nobody → jdescottes
Status: NEW → ASSIGNED

(Clearing the ni, thanks for the review!)

Flags: needinfo?(smaug)
Depends on: 1999229

Comment on attachment 9525353 [details]
Bug 1855045 - Emit observer notification when input type file triggers a filepicker

Revision D271790 was moved to bug 1999229. Setting attachment 9525353 [details] to obsolete.

Attachment #9525353 - Attachment is obsolete: true
Blocks: 1999693
Attachment #9525356 - Attachment description: Bug 1855045 - [wdspec] Add input file_dialog_opened test for input.showFilePicker API → Bug 1855045 - [wdspec] Add input file_dialog_opened test for input.showPicker API
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/56015 for changes under testing/web-platform/tests
Whiteboard: [webdriver:m18] → [webdriver:m18], [wptsync upstream]
Upstream PR merged by moz-wptsync-bot
Upstream PR merged by moz-wptsync-bot
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → 147 Branch
Blocks: 2005774
Regressions: 2006012
Blocks: 1938333
Whiteboard: [webdriver:m18], [wptsync upstream] → [webdriver:m18][wptsync upstream][webdriver:relnote]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: