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

Clicking on save button dispatches a MouseEvent, button location in element's saveButtonClientRect #10703

Conversation

squelart
Copy link
Contributor

@squelart squelart commented Feb 26, 2023

b9dad07

Clicking on save button dispatches a MouseEvent, button location in element's saveButtonClientRect
https://bugs.webkit.org/show_bug.cgi?id=252967
rdar://problem/105952126

Reviewed by Tim Nguyen.

Instead of sending a CustomEvent with some hand-picked details, a click on the save button now dispatches a full copy of the original MouseEvent, and the location of the (shadow) save button can be found the element's saveButtonClientRect property.

* Source/WebCore/html/HTMLAttachmentElement.cpp:
(WebCore::HTMLAttachmentElement::saveButtonClientRect const):
* Source/WebCore/html/HTMLAttachmentElement.h:
* Source/WebCore/html/HTMLAttachmentElement.idl:

Canonical link: https://commits.webkit.org/261101@main

f9644dc

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ›  gtk
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk1 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-wk2
βœ… πŸ›  tv-sim ❌ πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch-sim

@squelart squelart self-assigned this Feb 26, 2023
@squelart squelart added the HTML Editing For bugs in HTML editing support (including designMode and contentEditable). label Feb 26, 2023
@squelart squelart force-pushed the eng/attachment-save-event-details branch from b559cda to 977ead6 Compare February 27, 2023 01:34
@squelart squelart changed the title Add details in save's CustomEvent.details Add details in attachment's save button's CustomEvent Feb 27, 2023
@squelart
Copy link
Contributor Author

Dear reviewers: @nt1m suggested I should try to use a standard MouseEvent, and give the client another way to retrieve the save button's location after that. I'll update later, when I hear back from the main client...

@squelart squelart marked this pull request as draft February 28, 2023 07:39
@squelart squelart force-pushed the eng/attachment-save-event-details branch from 977ead6 to ab03e0d Compare March 1, 2023 04:37
@squelart squelart changed the title Add details in attachment's save button's CustomEvent Clicking on save button dispatches a MouseEvent, button location in element's saveButtonClientRect Mar 1, 2023
Copy link
Member

@nt1m nt1m left a comment

Choose a reason for hiding this comment

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

Looks good, would be nice to have test coverage

@nt1m
Copy link
Member

nt1m commented Mar 1, 2023

You can manually enable flags in tests fwiw: <!-- webkit-test-runner [AttachmentWideLayoutEnabled=True] -->

@squelart
Copy link
Contributor Author

squelart commented Mar 1, 2023

Tests to be added in rdar://105474304 .

@squelart squelart force-pushed the eng/attachment-save-event-details branch from ab03e0d to 7feb57b Compare March 1, 2023 05:01
@squelart squelart requested a review from nt1m March 1, 2023 05:02
Source/WebCore/html/HTMLAttachmentElement.cpp Outdated Show resolved Hide resolved
Source/WebCore/html/HTMLAttachmentElement.cpp Outdated Show resolved Hide resolved
Source/WebCore/html/HTMLAttachmentElement.cpp Outdated Show resolved Hide resolved
@squelart squelart force-pushed the eng/attachment-save-event-details branch from 7feb57b to 176a7b7 Compare March 1, 2023 06:07
@squelart squelart requested a review from nt1m March 1, 2023 06:09
@squelart squelart marked this pull request as ready for review March 1, 2023 23:24
auto copiedEvent = MouseEvent::create(
m_attachment->attributeWithoutSynchronization(saveAttr()), Event::CanBubble::No, Event::IsCancelable::No, Event::IsComposed::No,
mouseEvent.view(), mouseEvent.detail(), mouseEvent.screenX(), mouseEvent.screenY(), mouseEvent.clientX(), mouseEvent.clientY(),
mouseEvent.modifierKeys(), mouseEvent.button(), mouseEvent.buttons(), mouseEvent.syntheticClickType(), mouseEvent.relatedTarget());
Copy link
Member

Choose a reason for hiding this comment

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

relatedTarget() should probably be nullptr, see https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/relatedTarget

@squelart squelart force-pushed the eng/attachment-save-event-details branch from 176a7b7 to f9644dc Compare March 1, 2023 23:35
@nt1m nt1m added the merge-queue Applied to send a pull request to merge-queue label Mar 2, 2023
…lement's saveButtonClientRect

https://bugs.webkit.org/show_bug.cgi?id=252967
rdar://problem/105952126

Reviewed by Tim Nguyen.

Instead of sending a CustomEvent with some hand-picked details, a click on the save button now dispatches a full copy of the original MouseEvent, and the location of the (shadow) save button can be found the element's saveButtonClientRect property.

* Source/WebCore/html/HTMLAttachmentElement.cpp:
(WebCore::HTMLAttachmentElement::saveButtonClientRect const):
* Source/WebCore/html/HTMLAttachmentElement.h:
* Source/WebCore/html/HTMLAttachmentElement.idl:

Canonical link: https://commits.webkit.org/261101@main
@webkit-commit-queue
Copy link
Collaborator

Committed 261101@main (b9dad07): https://commits.webkit.org/261101@main

Reviewed commits have been landed. Closing PR #10703 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit b9dad07 into WebKit:main Mar 2, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HTML Editing For bugs in HTML editing support (including designMode and contentEditable).
Projects
None yet
4 participants