Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable 'accept' attribute in 'input' label. #2200

Merged
merged 2 commits into from
Jul 16, 2015
Merged

Conversation

hokein
Copy link
Contributor

@hokein hokein commented Jul 13, 2015

Fixes #1768, #1780.

@@ -56,10 +108,13 @@ void WebDialogHelper::RunFileChooser(content::WebContents* web_contents,
}

std::vector<base::FilePath> paths;
auto browser_context = AtomBrowserMainParts::Get()->browser_context();
Copy link
Member

Choose a reason for hiding this comment

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

We should use the browser_context of current WebContents.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current implementation relies on the PrefService in brightray::BrowserContext, which is aligned with Chromium's. And content::BrowserContext doesn't has such interface.

I know current implementation will cause <input> using the last used directory even in different WebContents, but this behavior is same with Chromium's(Chromium will share this last used directory to all WebContents in the same Profile).

Copy link
Member

Choose a reason for hiding this comment

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

The current implementation relies on the PrefService in brightray::BrowserContext, which is aligned with Chromium's. And content::BrowserContext doesn't has such interface.

We can get it from current window.

I know current implementation will cause using the last used directory even in different WebContents, but this behavior is same with Chromium's(Chromium will share this last used directory to all WebContents in the same Profile).

Chrome's Profile is equivalent to Electron's Session, both of them inherit from content::BrowserContext.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can get it from current window.

I didn't find any APIs in NativeWindow. 😢

Copy link
Member

Choose a reason for hiding this comment

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

NativeWindow is a WebContentsObserver so you can call web_contents().

Copy link
Member

Choose a reason for hiding this comment

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

Actually all content::BrowserContext created in Electron should be an AtomBrowserContext, it is safe to call static_cast.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@zcbenz
Copy link
Member

zcbenz commented Jul 16, 2015

👍

zcbenz added a commit that referenced this pull request Jul 16, 2015
Enable 'accept' attribute in 'input' label.
@zcbenz zcbenz merged commit 02cadde into master Jul 16, 2015
@zcbenz zcbenz deleted the input_accept_attr branch July 16, 2015 14:20
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.

None yet

2 participants