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

A minor fix for sendInputEvent and improvements related to cursor-changed event #6661

Merged
merged 7 commits into from Aug 1, 2016

Conversation

Projects
None yet
2 participants
@brenca
Member

brenca commented Jul 31, 2016

Fixes and improvements for the following:

  • Because of some changes, we were unable to send UTF-8 characters with sendInputEvent
  • The cursor-changed event was missing some data needed for custom cursors (hotspot + size of cursor)
  • To be able to draw the cursor easily, I introduced a ToBUFFER to NativeImage that retrieves the image in a raw representation into a NodeBuffer
@@ -1299,6 +1299,8 @@ void WebContents::OnCursorChange(const content::WebCursor& cursor) {
if (cursor.IsCustom()) {
Emit("cursor-changed", CursorTypeToString(info),
gfx::Image::CreateFrom1xBitmap(info.custom_image),
gfx::Rect(info.custom_image.width(), info.custom_image.height()),

This comment has been minimized.

@zcbenz

zcbenz Jul 31, 2016

Contributor

We should use gfx::Size here.

@@ -1299,6 +1299,8 @@ void WebContents::OnCursorChange(const content::WebCursor& cursor) {
if (cursor.IsCustom()) {
Emit("cursor-changed", CursorTypeToString(info),
gfx::Image::CreateFrom1xBitmap(info.custom_image),
gfx::Rect(info.custom_image.width(), info.custom_image.height()),
info.hotspot,

This comment has been minimized.

@zcbenz

zcbenz Jul 31, 2016

Contributor

The new arguments should be placed after info.image_scale_factor, otherwise it would be a breaking change. And can you mention the new arguments in docs/web-contents.md?

@@ -350,6 +359,7 @@ void NativeImage::BuildPrototype(
mate::ObjectTemplateBuilder(isolate, prototype)
.SetMethod("toPNG", &NativeImage::ToPNG)
.SetMethod("toJPEG", &NativeImage::ToJPEG)
.SetMethod("toBUFFER", &NativeImage::ToRawBuffer)

This comment has been minimized.

@zcbenz

zcbenz Jul 31, 2016

Contributor

We should probably use toBitmap as name, since other APIs are also returning Buffers.

out->unmodifiedText[0] = str[0];
str.size() <= 2) {
base::string16 code = base::UTF8ToUTF16(str);
out->text[0] = code[0];

This comment has been minimized.

@zcbenz

zcbenz Jul 31, 2016

Contributor

A character can take 4 bytes at maximum (for now), we should just ignore the size and copy all characters to text:

  // Make sure to not read beyond the buffer in case some bad code doesn't
  // NULL-terminate it (this is called from plugins).
  size_t text_length_cap = WebKeyboardEvent::textLengthCap;
  base::string16 text16 = base::UTF8ToUTF16(text);

  memset(out->text, 0, text_length_cap);
  memset(out->unmodifiedText, 0, text_length_cap);
  for (size_t i = 0; i < std::min(text_length_cap, text16.size()); ++i)
    out->text[i] = text16[i];
@brenca

This comment has been minimized.

Member

brenca commented Aug 1, 2016

@zcbenz I've corrected my bloopers and added documentation, thanks for the review :)

@zcbenz

This comment has been minimized.

Contributor

zcbenz commented Aug 1, 2016

👍

@zcbenz zcbenz merged commit 1d33275 into electron:master Aug 1, 2016

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment