Skip to content

Conversation

scheib
Copy link
Contributor

@scheib scheib commented Aug 19, 2025

No description provided.

Copy link
Collaborator

@lgarron lgarron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needed a make format, but looks good to me!

displayOutcome(
"open-file-picker",
"error",
)("window.showOpenFilePicker not available");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this logging is actually broken. I've pushed a separate fix: 144752c

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, checking in, this is resolved now?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, should be fixed.

index.html Outdated
<button id="eme">Encrypted Media (EME)</button>
<button id="idle-detection">Idle Detection</button>
<button id="persistent-storage">Persistent Storage</button>
<button id="open-file-picker">Open File Picker</button>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguably it might make sense to match the API names and use "show" instead of "open", but I personally don't feel strongly.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uh, I guess this more specifically applies to .showDirectoryPicker(…) (which does not contain "open").

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use method names verbatim.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@scheib
Copy link
Contributor Author

scheib commented Sep 11, 2025

Updates made, ready for re-review.

Copy link
Collaborator

@lgarron lgarron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@scheib scheib closed this Sep 11, 2025
@scheib scheib reopened this Sep 11, 2025
@scheib
Copy link
Contributor Author

scheib commented Sep 11, 2025

Thanks, I don't have close + squash access on that repo.

@lgarron
Copy link
Collaborator

lgarron commented Sep 11, 2025

Thanks, I don't have close + squash access on that repo.

I should be able to merge!

@lgarron lgarron merged commit 9119954 into chromium:master Sep 11, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants