Skip to content

Fix isMouseEvent returning false for simulated context menu events #301

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

Merged
merged 3 commits into from
Jan 29, 2021

Conversation

greglittlefield-wf
Copy link
Collaborator

@greglittlefield-wf greglittlefield-wf commented Jan 26, 2021

Motivation

isMouseEvent was incorrectly returning false for events created via Simulate.contextMenu.

It looks like we were missing type-checking test coverage for contextmenu events (I checked to see if any more were missing, but contextmenu was the only one).

Solution

  • Update type string checks in isMouseEvent so that it returns true for simulated context menu events
  • Update test coverage

QA Instructions

  • CI passes

@aviary-wf
Copy link

aviary-wf commented Jan 26, 2021

Security Insights

No security relevant content was detected by automated scans.

Action Items

  • Review PR for security impact; comment "security review required" if needed or unsure
  • Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on Slack: #support-infosec.

@semveraudit-wf
Copy link

semveraudit-wf commented Jan 26, 2021

Public API Changes

No changes to the public API found for commit f3f880f

Showing results for f3f880f

Powered by semver-audit-service. Please report any problems by filing an issue.
Reported by the dart semver audit client 2.2.0
Browse public API.

Last edited UTC Jan 27 at 16:45:09

@greglittlefield-wf greglittlefield-wf changed the title Fix simulated context menu events not being mouse events Fix isMouseEvent returning false for simulated context menu events Jan 27, 2021
Copy link
Collaborator

@joebingham-wk joebingham-wk left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for fixing this, Greg! I can upgrade to a plus ten once CI passes

Copy link
Collaborator

@joebingham-wk joebingham-wk left a comment

Choose a reason for hiding this comment

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

QA +1

  • CI passing (dev run is expected to fail)

@aaronlademann-wf
Copy link
Collaborator

@aviary-wf try again please

@greglittlefield-wf
Copy link
Collaborator Author

@aviary-wf

@greglittlefield-wf
Copy link
Collaborator Author

@Workiva/release-management-p

Copy link

@rmconsole-wf rmconsole-wf left a comment

Choose a reason for hiding this comment

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

+1 from RM

@rmconsole5-wk rmconsole5-wk merged commit 054a177 into master Jan 29, 2021
@rmconsole5-wk rmconsole5-wk deleted the context-menu-is-mouse-event branch January 29, 2021 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants