Added CefRequestHandler.OnBeforePluginLoad #318

fraenke75 · 2014-04-12T06:53:47Z

Added the missing method implementation for CefRequestHandler.OnBeforePluginLoad. Extended the IRequestHandler interface.

jornh · 2014-04-13T12:32:30Z

@fraenke75 regarding the code I'd say this looks like a really good contribution. There are a few issues with whitespace which should be easy to spot if you look at https://github.com/cefsharp/CefSharp/pull/318/commits ah I mean https://github.com/cefsharp/CefSharp/pull/318/files. To have those pages look good we use spaces and not tabs. I'll point to a few of them inline.

You can just fix it in a followup commit and push that to your RequestHandlers branch and it will show up as part of this PR

jornh · 2014-04-13T12:36:05Z

CefSharp.Core/Internals/CefWebPluginInfoWrapper.cpp

+{
+    namespace Internals
+    {
+		String^ CefWebPluginInfoWrapper::Description::get()


Whitespace problem

fraenke75 · 2014-04-14T14:29:08Z

@jornh: You are right. I usually program in C# and have the VS-setting to use spaces and not tabs there. Unfortunately I missed to set this for C++ too :-)

fraenke75 · 2014-04-14T14:30:06Z

Sorry wrong usage of GitHub.

Reopened again.

jornh · 2014-04-14T14:46:54Z

... in C# and have the VS-setting to use spaces and not tabs there. Unfortunately I missed to set this for C++ too :-)

Ahh yes ... everyone starting to make PR's seems to stumble on that one. Should really stand out in https://github.com/cefsharp/CefSharp/blob/master/CONTRIBUTING.md ... I guess PR's for that one is welcome too 😄 - as is stands now you are really met by a "wall of text" so its hard to find the important bit's in there. I know @amaitland did too

Maybe clear sections for people 'just' reporting issues and people submitting PR's would help?

Sorry wrong usage of GitHub. Reopened again.

LOL ... you can only learn by pulling some of the handles eh?

fraenke75 · 2014-04-14T15:10:01Z

LOL ... you can only learn by pulling some of the handles eh?

My time is currently rather low (who's not?), but I will try to keep on supporting CEFSharp. I have already seen some handlers which I will implement.

This reverts commit d5b2cd3.

perlun · 2014-04-20T14:44:02Z

CefSharp.Example/ExamplePresenter.cs

@@ -49,10 +49,10 @@ public static void Init()
        private readonly IWebBrowser model;
        private readonly Action<Action> uiThreadInvoke;

-        public ExamplePresenter(IWebBrowser model, Action<Action> uiThreadInvoke)
+        public ExamplePresenter(IWebBrowser model) //, Action<Action> uiThreadInvoke)


Commented out code I do not like... 😄

jornh · 2014-05-13T21:22:33Z

Could you please add a small usage example description either as this: #342 (comment) or for added bonus even as an example one can invoke from the menu of either the Wpf or WinForms example?

Sorry for moving the entry bar a bit up here 😄 but in in the long run it will benefit everybody in getting to know CefSharp!

fraenke75 · 2014-05-22T15:42:42Z

Usage:

Create a Presenter class which inherits from IIRequestHandler

public class ExamplePresenter : IRequestHandler
{
    //Implement all IRequestHandler interfaces

    //Implement OnBeforePluginLoad
    bool IRequestHandler.OnBeforePluginLoad(IWebBrowser browser, string url, string policy_url, IWebPluginInfo info)
    {
        return !info.Name.ToLower().Contains("flash");   //Do not allow to load a plugin which has "flash" in its name
    }
}

In ViewModel (or anywhere else) where Webbrowser is loaded attach the Presenter class to the Webbrowser instance:

ExamplePresenter presenter = new ExamplePresenter(WebBrowser, (action) => Application.Current.Dispatcher.BeginInvoke(action));

Hope this helps.

jornh · 2014-05-22T15:57:45Z

It certainly does thank you very much!

In return I'll take it on my shoulders to merge in master before I will take it for a spin - and merge it. We didn't really merge it when we could and suddenly ended up with a lot of PR's competing for our attention. That is, unless you want to practice your skills with Git a bit too with that ... and have the time for it? 😜

Added CefRequestHandler.OnBeforePluginLoad

jornh · 2014-05-24T19:37:54Z

Uh-ooh, that didn't turn out as planned! I somehow ended up committing my manual merge plus a few minor fixes directly to master so it's not linked to this PR at all: 3d6a52e...31cc615

@perlun do you have any suggestions on what I should do here? I of courses wanted @fraenke75's PR to be part of it. I think I have done one of these manual merges successfully in SourceTree/terminal before ... but this one for some reason said this

jhe@W57167 ~/GitHub/jornh/CefSharp (master)
$ git merge --no-ff fraenke75-RequestHandlers
Already up-to-date.

I'm really sorry guys! I tried my best 😢

perlun · 2014-05-24T20:14:56Z

Don't worry Jørn! I stripped away your changesets (by using the reset command in SourceTree, i.e this:

git -c diff.mnemonicprefix=false -c core.quotepath=false -c credential.helper=sourcetree reset -q --hard 3d6a52e5c9a69a1506f80a2b573628c434c5e9b5 
Completed successfully

So, you can now retry the merge & manual fixes etc.

jornh · 2014-05-24T20:18:35Z

Cool thanks mate! I'd hate having mess in master because of me.

Will have another careful go at it - but not today. Better cool down a bit

amaitland · 2014-05-26T01:37:22Z

@jornh Did you make any progress with this? Any chance you could post a quick HOWTO when you've got it sorted? Is it as simple as merge the PR into local master, resolve conflicts, commit, then push?

amaitland · 2014-05-26T03:06:49Z

CefSharp.Core/Internals/ClientAdapter.h

@@ -71,6 +71,7 @@ namespace CefSharp
            virtual DECL bool OnBeforeResourceLoad(CefRefPtr<CefBrowser> browser, CefRefPtr<CefFrame> frame, CefRefPtr<CefRequest> request) OVERRIDE;
            virtual DECL bool GetAuthCredentials(CefRefPtr<CefBrowser> browser, CefRefPtr<CefFrame> frame, bool isProxy,
                const CefString& host, int port, const CefString& realm, const CefString& scheme, CefRefPtr<CefAuthCallback> callback) OVERRIDE;
+            virtual bool OnBeforePluginLoad( CefRefPtr< CefBrowser > browser, const CefString& url, const CefString& policy_url, CefRefPtr< CefWebPluginInfo > info );


Noticed this was slightly inconsistent, should it be this below?

virtual DECL bool OnBeforePluginLoad(CefRefPtr<CefBrowser> browser, const CefString& url, const CefString& policy_url, CefRefPtr<CefWebPluginInfo> info) OVERRIDE;

I changed that during my review here: jornh@7265a0d#diff-7bb59914aa140dd1e2255adc0b82c204R86

Perfect! 😄

jornh · 2014-05-26T05:01:32Z

@jornh Did you make any progress with this? Any chance you could post a quick HOWTO when you've got it sorted? Is it as simple as merge the PR into local master, resolve conflicts, commit, then push?

Yes, like the last 3 commits in this? https://github.com/jornh/CefSharp/compare/fraenke75-RequestHandlers

One thing I would like both of you @fraenke75 and @amaitland to review is that I changed the meaning of the bool return value to the opposite (but that's what my testing - and the CEF API docs suggested)

jornh · 2014-05-26T05:21:07Z

@fraenke75 I submitted the merge and my two change commits as a PR towards your branch: fraenke75#1

I newer tried it this way around before (I'm still now to git/GitHub too 😄) but with a bit of luck it should add it to your branch just by pushing the green merge button AND this #318 PR and we should get a green merge button too

amaitland · 2014-05-26T05:25:11Z

/// Return true to block loading of the plugin.

@jornh That's consistent with other functions exposed via IRequestHandler 👍

I can run it to double check if you'd like? Though I'm sure it works as advertised 😄

jornh · 2014-05-26T05:33:55Z

Yes I noted that too like CEF says "return false in your handler if you don't want to change my default"

@ You're welcome to take it for a spin. Or sit tight and give @fraenke75 a bit of time to look at it.

amaitland · 2014-05-26T12:26:14Z

Will take it for a spin and let you know 😄

amaitland · 2014-05-26T13:10:44Z

@jornh Works exactly as advertised!

jornh · 2014-05-26T13:26:44Z

@amaitland I'll take that together with your comments on other issues that you are waiting for #318 as a vote to merge #318 plus my additions in fraenke75#1 now 😄 Thanks for giving it an extra set of eyes!

amaitland · 2014-05-26T13:41:36Z

@jornh once your changes have been merged into #318 then it has my vote to be merged next 😄 Still haven't hit your target of having under 10 open PR's yet, so we'll just have to keep working at it! Plus I've got some changes in the pipeline!

amaitland · 2014-05-26T13:51:31Z

Awesome! The list grows smaller! 😄

@jornh Was there a secret to merging so that github recognised this PR?

jornh · 2014-05-26T14:23:56Z

Ahem ashamed to say it, but I'd better let it out, I think the problem was: "When following the recipe below - make sure you actually commit (at the right point) after manual resolution of the merge conflicts" 😝 Good thing @perlun knew where to find the brakes!

Step 1: From your project repository, check out a new branch and test the changes.

git checkout -b <user>-<branchname> master
git pull https://github.com/<user>/CefSharp.git <branchname>

Step 2: Merge the changes and update on GitHub.

git checkout master
git merge --no-ff <user>-<branchname>
git push origin master

Anyway @fraenke75 thanks a bunch for your good contribution 👍 much appreciated! Happy to see more if you find another pet project. I'll close the PR towards your fork in a few days ... feel free to experiment with it otherwise.

Added CefRequestHandler.OnBeforePluginLoad

413567a

jornh reviewed Apr 13, 2014
View reviewed changes

Fixed whitespaces issue

51608c1

fraenke75 closed this Apr 14, 2014

fraenke75 reopened this Apr 14, 2014

fraenke75 added 2 commits April 15, 2014 11:55

Added BrowserSettings and removed Nullable<bool>

d5b2cd3

Revert "Added BrowserSettings and removed Nullable<bool>"

a7ec1b1

This reverts commit d5b2cd3.

jornh mentioned this pull request Apr 19, 2014

geolocation support #332

Closed

jornh added the enhancement label Apr 19, 2014

perlun added not-ready-for-merge and removed not-ready-for-merge labels Apr 20, 2014

perlun reviewed Apr 20, 2014
View reviewed changes

perlun added the not-ready-for-merge label Apr 20, 2014

fraenke75 added 2 commits April 22, 2014 09:06

Reverted changes in Visual Studio solution file

8d1809d

Reverted changes which should not be committed

1b66115

Added the usage of the OnBeforePluginLoad to the ExamplePresenter

22c1cce

jornh removed the not-ready-for-merge label May 24, 2014

jornh added a commit that referenced this pull request May 24, 2014

Merge pull request #318 from fraenke75/RequestHandlers

36f0838

Added CefRequestHandler.OnBeforePluginLoad

amaitland reviewed May 26, 2014
View reviewed changes

amaitland mentioned this pull request May 26, 2014

CEF3: Fix the WinForms example to be a tabbed browser #279

Closed

jornh merged commit 22c1cce into cefsharp:master May 26, 2014

jornh added this to the 31.0.0 milestone May 26, 2014

jornh mentioned this pull request Oct 31, 2014

Spellchecker? #222

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added CefRequestHandler.OnBeforePluginLoad #318

Added CefRequestHandler.OnBeforePluginLoad #318

fraenke75 commented Apr 12, 2014

jornh commented Apr 13, 2014

jornh Apr 13, 2014

fraenke75 commented Apr 14, 2014

fraenke75 commented Apr 14, 2014

jornh commented Apr 14, 2014

fraenke75 commented Apr 14, 2014

perlun Apr 20, 2014

jornh commented May 13, 2014

fraenke75 commented May 22, 2014

jornh commented May 22, 2014

jornh commented May 24, 2014

perlun commented May 24, 2014

jornh commented May 24, 2014

amaitland commented May 26, 2014

amaitland May 26, 2014

jornh May 26, 2014

amaitland May 26, 2014

jornh commented May 26, 2014

jornh commented May 26, 2014

amaitland commented May 26, 2014

jornh commented May 26, 2014

amaitland commented May 26, 2014

amaitland commented May 26, 2014

jornh commented May 26, 2014

amaitland commented May 26, 2014

amaitland commented May 26, 2014

jornh commented May 26, 2014

Added CefRequestHandler.OnBeforePluginLoad #318

Added CefRequestHandler.OnBeforePluginLoad #318

Conversation

fraenke75 commented Apr 12, 2014

jornh commented Apr 13, 2014

jornh Apr 13, 2014

Choose a reason for hiding this comment

fraenke75 commented Apr 14, 2014

fraenke75 commented Apr 14, 2014

jornh commented Apr 14, 2014

fraenke75 commented Apr 14, 2014

perlun Apr 20, 2014

Choose a reason for hiding this comment

jornh commented May 13, 2014

fraenke75 commented May 22, 2014

jornh commented May 22, 2014

jornh commented May 24, 2014

perlun commented May 24, 2014

jornh commented May 24, 2014

amaitland commented May 26, 2014

amaitland May 26, 2014

Choose a reason for hiding this comment

jornh May 26, 2014

Choose a reason for hiding this comment

amaitland May 26, 2014

Choose a reason for hiding this comment

jornh commented May 26, 2014

jornh commented May 26, 2014

amaitland commented May 26, 2014

jornh commented May 26, 2014

amaitland commented May 26, 2014

amaitland commented May 26, 2014

jornh commented May 26, 2014

amaitland commented May 26, 2014

amaitland commented May 26, 2014

jornh commented May 26, 2014