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

fix: honor cursor blink rate #20020

Merged
merged 5 commits into from
Aug 30, 2019
Merged

fix: honor cursor blink rate #20020

merged 5 commits into from
Aug 30, 2019

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Aug 29, 2019

Description of Change

Closes #10668.

Ensures that WebPreferences honor the system cursor blink rate on creation.

cc @zcbenz @nornagon @MarshallOfSound

Checklist

Release Notes

Notes: Fixed an issue where cursor blink rate was not honored.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Aug 29, 2019
@codebytere codebytere changed the title fix: honor cursor blink rate on macOS fix: honor cursor blink rate on macOS/Linux Aug 29, 2019
Copy link
Contributor

@miniak miniak left a comment

Choose a reason for hiding this comment

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

Can you please implement it on Windows using GetCaretBlinkTime?

Copy link
Contributor

@miniak miniak left a comment

Choose a reason for hiding this comment

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

Or actually this seems better
https://cs.chromium.org/chromium/src/chrome/browser/renderer_preferences_util.cc?type=cs&q=GetCursorBlinkInterval&sq=package:chromium&g=0&l=123-125

#if defined(TOOLKIT_VIEWS)
  prefs->caret_blink_interval = views::Textfield::GetCaretBlinkInterval();
#endif

@codebytere codebytere changed the title fix: honor cursor blink rate on macOS/Linux fix: honor cursor blink rate Aug 29, 2019
@codebytere codebytere requested a review from miniak August 29, 2019 15:12
@codebytere codebytere force-pushed the honor-cursor-mac branch 3 times, most recently from a112048 to 8fa05b0 Compare August 29, 2019 16:28
Copy link
Member

@ckerr ckerr left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the updates!

@miniak
Copy link
Contributor

miniak commented Aug 29, 2019

@codebytere can we use views::Textfield::GetCaretBlinkInterval();? it wraps GetCaretBlinkTime

@codebytere
Copy link
Member Author

@miniak it does wrap that method, but it also wraps things for mac and so for clarity & maintainability i’d prefer we explicitly set all three ourselves since we’d need to call the linux one separately regardless.

Perhaps we could consider upstreaming the linux one into that call and then using that in the future?

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Aug 30, 2019
@ckerr ckerr merged commit 5cbbd48 into master Aug 30, 2019
@release-clerk
Copy link

release-clerk bot commented Aug 30, 2019

Release Notes Persisted

Fixed an issue where cursor blink rate was not honored.

@ckerr ckerr deleted the honor-cursor-mac branch August 30, 2019 14:39
@trop
Copy link
Contributor

trop bot commented Aug 30, 2019

I have automatically backported this PR to "7-0-x", please check out #20044

@trop trop bot mentioned this pull request Aug 30, 2019
@trop trop bot removed the target/7-0-x label Aug 30, 2019
@trop trop bot mentioned this pull request Aug 30, 2019
@trop trop bot added the in-flight/7-0-x label Aug 30, 2019
@trop trop bot mentioned this pull request Aug 30, 2019
@trop
Copy link
Contributor

trop bot commented Aug 30, 2019

I have automatically backported this PR to "5-0-x", please check out #20045

@trop
Copy link
Contributor

trop bot commented Aug 30, 2019

I have automatically backported this PR to "6-0-x", please check out #20046

@sofianguy sofianguy added this to Fixed in 7.0.0-beta.4 in 7.2.x Sep 3, 2019
@sofianguy sofianguy added this to Fixed for 6.0.8 in 6.1.x Sep 18, 2019
@sofianguy sofianguy added this to Fixed in 5.0.11 in 5.0.x Sep 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
5.0.x
Fixed in 5.0.11
6.1.x
Fixed for 6.0.8
7.2.x
Fixed in 7.0.0-beta.4
Development

Successfully merging this pull request may close these issues.

Honor the OS's cursor blink settings
5 participants