Implemented all CEF3 BrowserSettings #326
Conversation
Added missing Browser Settings and replaced the Nullable<bool> with a human readable BrowserSettingsState like in the CEF3
CefSharp.Core/BrowserSettings.h
Outdated
There was a problem hiding this comment.
No regions please. :) @jornh, can you please add this to the contributions guidelines?
|
Anyway, the rest of the change (= supporting all settings) is good so let's just try to decide on these things and we can then try to merge this in the near future. |
|
Maybe the introduction of a new enum is not necessary here. In the past (CEF1) there were xxxDisabled and xxxEnabled settings, but they do not exist any more in CEF3. |
|
Yes, go with that. But I think it should be xxxDisabled sometimes and xxxEnabled sometimes, depending on the setting. Right? |
|
No, we can decide if we want to use only xxxEnabled or only xxxDisabled. Due to the modified interface in CEF3 the setting itself has no postfix. The enabled/disabled state is set by the enum. I would propose to use as name always the xxxDisabled. |
The used boolean setting names now match the command-line options.
CefSharp.Core/BrowserSettings.h
Outdated
There was a problem hiding this comment.
The name here should be JavascriptDisabled; we don't capitalize the S.
There was a problem hiding this comment.
In general, I think the "disabled" suffix is good, especially since (as you said) the command line options have the same kind of naming.
|
Looks good! Please fix these minor suggestions and we're good to merge. (@jornh, you may do it once they are done if I don't show up. 😃) |
|
Finally able to get back to this one. Thanks @fraenke75 and welcome as a CefSharp committer 😄 |
3 participants
Implement all CEF3 settings which are defined in cef_browser_settings_t struct.
Changed the Nullable to an enumeration _BrowserSettingsState like in CEf3. I think this is easier to read instead of the multiple mixture of xxxDisable/xxxEnabled settings and the assigned bool.