pottermm / javachromiumembedded

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

Windows: Add windowed rendering support #32

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1.
2.
3.

What is the expected output? What do you see instead?
After adding windowed rendering support to Mac, I've added it to Windows, too. 
Please see Issue #31 for details of the Mac implementation.

What version of the product are you using? On what operating system?

Please provide any additional information below.

Please note: This patch relies on the attached patch file of issue #31. 
If you want to try these code changes, add the patch of issue #31 first and 
afterwards add the patch file attached to this issue. Otherwise you'll get some 
patch-errors.

Original issue reported on code.google.com by k...@censhare.de on 22 Jan 2014 at 1:51

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks for the patch. Some comments:

1. line 164:

+    if(CefContext.isMacintosh() ) {

Style says this should be "if (CefContext.isMacintosh()) {". Same in other 
places.

2. line 422:

+  public void onTakeFocus(CefClient client, boolean next) {
+    if( next ) {
+      backButton_.requestFocus();
+    }
+    else {
+      goButton_.requestFocus();
+    }
+  }

Instead of manually programming the tab order is there a way to pass the event 
to Java and have it focus the next control automatically?

3. line 462:

+    HWND handle = browser->GetHost()->GetWindowHandle();
+    SetWindowPos(handle, NULL, 1, 1, width-1, height-1, SWP_NOZORDER | 
SWP_NOMOVE);

Why the 1 pixel offset?

4. line 480:

+  browser->GetHost()->SetFocus(enable == JNI_TRUE);
+  browser->GetHost()->SendFocusEvent(enable == JNI_TRUE);

SetFocus is for windowed browsers and SendFocusEvent is for osr browsers. You 
shouldn't call both.

Original comment by magreenb...@gmail.com on 22 Jan 2014 at 4:40

GoogleCodeExporter commented 9 years ago
Thanks for reviewing the patch.
Regarding your questions and comments:

1.) Yes, styling is fixed now (see Issue 31)

2.) Yes, I've found a a way to do this. I changed the code to use
the FocusTraversalPolicy which is responsible for the traversal chain.
From this policy object I get the first/last object in the chain to pass
the focus over.

3.) Sorry that sliped in. This was only a testcase on my side and I've removed 
the offset now.

4.) Thanks for the hint. I didn't knew that. I've added an if-clause to it.

See attached patch-file for my changes.
Please note that this patch file depends on the patch from issue #31.

Original comment by k...@censhare.de on 24 Jan 2014 at 11:08

Attachments:

GoogleCodeExporter commented 9 years ago
Sorry, but I forgot to add a java include to MainFrame.java.

So, here is a new version of the patch file.

Original comment by k...@censhare.de on 27 Jan 2014 at 8:25

Attachments:

GoogleCodeExporter commented 9 years ago
Added in revision 22 with minor style and documentation changes changes.

I've disabled windowed rendering by default on Windows because I'm getting the 
following crash while resizing the window: 
http://magpcss.org/ceforum/viewtopic.php?f=6&t=10415. We'll need to fix this 
before enabling windowed rendering by default.

Original comment by magreenb...@gmail.com on 30 Jan 2014 at 7:04

GoogleCodeExporter commented 9 years ago
@comment#4: The resize issue has been fixed by the CEF 1750 branch update. 
Revision 30 enables windowed mode by default.

Original comment by magreenb...@gmail.com on 20 Feb 2014 at 5:13