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

Support for manually specifying command line args via ::OnCommandLineProcessing #365

Merged
merged 4 commits into from May 30, 2014

Conversation

Timobile
Copy link
Contributor

…om within the app and be processed by cef (used OnBeforeCommandLineProcessing for implementation)
@@ -13,16 +13,50 @@ namespace CefSharp
class CefSharpApp : public CefApp
{
gcroot<CefSettings^> _cefSettings;
gcroot<IDictionary<String^, String^>^> _cefCommandLineArgs;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i am new to c++/cli, did look up gcnew and gcroot but i am unsure if this is right

@Timobile
Copy link
Contributor Author

to add proxy settings you could do this:

var commandLineArgs = new Dictionary<string, string>
{
{"proxy-server", "http=webfilter.mydomain.com:8080" },
{"proxy-bypass-list", ".google.;*.local"}
}

@JanEggers
Copy link
Contributor

nicely done i will try that out on monday with our corporate proxy.

i think it would be cleaner if the Dictionary was added as property to the cefsettings.

@jornh jornh added this to the 31.0.0 milestone May 23, 2014
@Timobile
Copy link
Contributor Author

ok, i will change this as soon as possible.

@Timobile
Copy link
Contributor Author

i think that all/most of the other properties of CefSettings.h are "forwarded" to a native struct/a type wrapper from the cef.sdk. I don't know how i should add command line args there. Or is it enough to stay at the clr side and not forward these to the struct? I think i have to look closer at the code. At the moment i do not understand what is happening there.

@JanEggers
Copy link
Contributor

just stay at the clr side at use it where u need it

@Timobile
Copy link
Contributor Author

did update my pr and did incorporate command line args into CefSettings

@amaitland
Copy link
Member

@Timobile Adding CommandLineArgs to CefSettings looks much cleaner, nice work 👍

@amaitland
Copy link
Member

Perhaps it's worth posting an updated usage example?

@Timobile
Copy link
Contributor Author

Perhaps it's worth posting an updated usage example?

sure, here it is:

settings.CefCommandLineArgs.Add("user-agent", "me");

right now you can not specify a whole dictionary to be used as CefCommandLineArgs. Should i add a setter for CefCommandLineArgs? Then it would look like above and additionally it would be possible to add many arguments like this:

settings.CefCommandLineArgs = new Dictionary<string, string>
{
    {"user-agent", "me"},
    {"proxy-server", "http=webfilter.mydomain.com:8080" }
};

@amaitland
Copy link
Member

@Timobile I don't really mind one way or another, so see what other people have as a preference. As it stands it's consistent with cefCustomSchemes.

@Timobile
Copy link
Contributor Author

yes, this was my intention when not adding the setter, but perhaps it is more convenient for the user to be able to use both possibilities? And probably you will add more than one commandLineArg at once. With schemes this is not the case i think (but i did never use them i have to admit).

@@ -1,4 +1,4 @@
// Copyright � 2010-2014 The CefSharp Authors. All rights reserved.
// Copyright � 2010-2014 The CefSharp Authors. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

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

Copyright symbol has become garbled, will need to fix at some stage.

Copy link
Contributor

Choose a reason for hiding this comment

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

@amaitland @perlun I'm thinking we should just replace the ©️ symbol with a (c) at some point even tough they look nice when they are not broken. They seem to break all the time. I think we can use our and others time better on something else. Agree?

Copy link
Member

Choose a reason for hiding this comment

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

Yep, works for me!

Copy link
Member

Choose a reason for hiding this comment

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

It's a shame we still fight the ASCII monster these days... Do you have any clue why it breaks? Is it the GitHub web UI that causes it? I think Visual Studio should be UTF8-safe.

Sent from my iPhone

On 30 maj 2014, at 10:40, "jornh" <notifications@github.commailto:notifications@github.com> wrote:

In CefSharp.Core/Internals/CefSharpApp.h:

@@ -1,4 +1,4 @@
-// Copyright ? 2010-2014 The CefSharp Authors. All rights reserved.
+// Copyright ? 2010-2014 The CefSharp Authors. All rights reserved.

@amaitlandhttps://github.com/amaitland @perlunhttps://github.com/perlun I'm thinking we should just replace the [:copyright:] symbol with a (c) at some point even tough they look nice when they are not broken. They seem to break all the time. I think we can use our and others time better on something else. Agree?

Reply to this email directly or view it on GitHubhttps://github.com//pull/365/files#r13219264.

@amaitland
Copy link
Member

I've had a chance to test this one, other than the two minor pedantic things I'm happy to see this one merged 😄

@jornh Anything else you'd like to see before merging this one? I don't manually fixing up this two minor issues
@Timobile Thanks for the great work on this one!

@jornh
Copy link
Contributor

jornh commented May 30, 2014

No I think it's good! Please go ahead and do the merge.

@Timobile a VERY well done and useful first contribution 😄 Happy to see more from your side, Vielen Dank!

amaitland added a commit that referenced this pull request May 30, 2014
Support for manually specifying command line args via ::OnCommandLineProcessing
@amaitland amaitland merged commit 10e4233 into cefsharp:master May 30, 2014
@amaitland
Copy link
Member

@jornh I've fixed those two small things on master so I think we can happily move on to the next PR 😄

@amaitland
Copy link
Member

When I think about it, we probably don't want to set "user-agent" to "me" in either of the examples, so I've commented that out and added a comment to serve as a usage example for the short term.
1415075

@jornh
Copy link
Contributor

jornh commented May 30, 2014

Good minor tweaks, @amaitland remember to close #239 (for people who listen to that one) with a reference to this PR and add a line to the ChangeLog (for everyone) - if you didn't already.

@Timobile
Copy link
Contributor Author

Timobile commented Jun 2, 2014

wow, cool, this got merged!
sorry that i did not show up last days - i just became father 2nd time and so life is flying by without ever looking at my emails.

The thing with the garbled copyright could realy been done by the web frontend at github. Changed something there and after that the update/push became 'difficult'. I think i did overlook this problem.

@jornh thanks for the encouragement, Gern' geschehen :-)
@amaitland the usage example was not very elaborated i guess. Did just throw in my test example which can easily be verified when looking at the data sent out. I think it's better the way you corrected it. Thanks.

@jornh
Copy link
Contributor

jornh commented Jun 2, 2014

No worries, family expansions always comes higher in on the list than Pull Request in CefSharp-land.

Big congrats on the new 🚼

@amaitland
Copy link
Member

@Timobile Congrats on the new 🚼

Thanks again for the contribution!

@kropewnicki
Copy link
Contributor

@Timobile if i wanted to pass in a named proxy using your settings, how would I do it?

for example if my network proxy is
from
https://code.google.com/p/chromiumembedded/wiki/GeneralUsage
command line switch is
--proxy-server="foopy:99"
I was thinking the the arg would be
string proxy = "="foopy:8080"";
settings.CefCommandLineArgs.Add("--proxy-server",proxy);

but it does not work the way i expect.

@jornh
Copy link
Contributor

jornh commented Jun 9, 2014

Try skipping the -- and also the = see #365 (comment) for a few examples.

@Timobile
Copy link
Contributor Author

Timobile commented Jun 9, 2014

@kropewnicki: yes, @jornh should be right. My Example above is

settings.CefCommandLineArgs.Add("user-agent", "me");

In your case try

settings.CefCommandLineArgs.Add("proxy-server", "foopy:99"); // this is for http and https

or

settings.CefCommandLineArgs.Add("proxy-server", "http=foopy:99"); // this is only for http

but be aware that my experience with handling of an (ntlm) authenticating proxy in the current version is that it does not work. Maybe it works if no ntlm authentication is required. I wrote some lines about it here #239 . If you get this working somehow, please leave a comment here or in #239 so i can try it with my setup too.

@oxbough
Copy link

oxbough commented Jun 10, 2014

I am back to working on a CEFsharp solution for my project; Thanks for adding this feature. I still get an exception when running in VS2010 debugger that stops me from running/testing/debugging the app that way. Maybe not related to this??? I thought it was. I even tried attaching the VS debugger to the app, and it ran fine until attaching the debugger. Same exception. (An event was unable to invoke any of the subscribers. Error Number 0x80040201)

@oxbough
Copy link

oxbough commented Jun 10, 2014

Looks like the issue I'm seeing is related to this ... https://connect.microsoft.com/VisualStudio/feedback/details/538649/system-runtime-interopservices-co

@oxbough
Copy link

oxbough commented Jun 10, 2014

OK. Found the solution for my issue. Found this info posted in 2010 about VS2010 ...
I had "break when exceptions cross AppDomain or managed/native boundaries" set in Option | Debug | General. Clear that flag and the exception stop presenting and you can debug as normally.

@perlun
Copy link
Member

perlun commented Jun 11, 2014

Another reason why we should perhaps drop VS2010 I guess. 😉 As has been discussed in #381 recently. 😛

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

7 participants