roadlabs / chromiumembedded

Automatically exported from code.google.com/p/chromiumembedded
0 stars 1 forks source link

Proposed chunk of changes #13

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Please find attached diff file with changes I propose to add.
There is a short description of changes:

1. Special method to create Browser synchronously was added 
(CreateBrowserSync)
2. Two functions to work with keyboard focus were added:
  - CefBrowser::SetFocus to set focus on Browser component
  - CefHandler::HandleTakeFocus - called when Chromium HTML component is 
about to loose focus (f.e. focus was on the last HTML element and user 
pressed TAB key).
3. CefRefPtr<CefBrowser>& alternateBrowser parameter was added to 
HandleBeforeCreated method - this will allow to return alternate browser 
for popup window. This workarounds problem with empty URLs that came into 
this method in case of popup windows.
4. CefHandler::HandleBeforeWindowClose method to catch a moment when popup 
window is about to be closed.

Look forward for your comments

Original issue reported on code.google.com by vrid...@gmail.com on 6 Feb 2009 at 2:19

Attachments:

GoogleCodeExporter commented 9 years ago
Overall, the proposed functionality looks good.  A few comments:

1. OnBeforeDestroyed() should probably be called in the BrowserWebViewDelegate
destructor instead of in Release().  A call to Release() does not mean that the
object has been destroyed -- for instance, Release() will be called every time a
smart pointer to the object goes out of scope.

2. All methods in CefHandler() should be pure virtual.

3. CefBrowser::CreateBrowserSync() should return NULL if the context does not 
exist
or if the current thread is not the UI thread.  So, in other words:

if(!_Context.get() || !_Context->RunningOnUIThread())
  return NULL;

Original comment by magreenb...@gmail.com on 10 Feb 2009 at 6:33

GoogleCodeExporter commented 9 years ago
Thank you for your comments
There is updated patch.
Except fixes for issues you have found, I added 2 more fixes:

1. In UIT_LoadURLForRequest I removed win32-specific code for understanding 
local 
files names, because GURL class can handle these cases by it's own. Also, I 
added 
code to add "http://" prefix if we were unable to parse URL - to be more 
consistent 
with other browsers (IE, Mozilla) behaviour. As far as UIT_LoadURLForRequest no 
longer contains platform-specific code, I moved it into browser_impl.cc

2. Short but significant fix in cefclient.cc:
-        wchar_t strPtr[MAX_URL_LENGTH];
+        wchar_t strPtr[MAX_URL_LENGTH] = {0};
In old version, URL was suffexed by garbage - this works for most http urls, 
but 
caused problems with "file://" protocol

Original comment by vrid...@gmail.com on 11 Feb 2009 at 1:29

Attachments:

GoogleCodeExporter commented 9 years ago
Rev 19 includes all of these changes except adding the CefRefPtr<CefBrowser>&
alternateBrowser parameter to HandleBeforeCreated().  I don't understand how 
adding
this parameter helps in working around the empty URL passed to 
HandleBeforeCreated().
 Can you provide an example?

Original comment by magreenb...@gmail.com on 8 Mar 2009 at 2:28

GoogleCodeExporter commented 9 years ago
This issue has been closed due to lack of response from the original author.

Original comment by magreenb...@gmail.com on 24 Mar 2011 at 8:48