[Site Isolation] Fix datetime picker positioning in cross-origin iframes#59667
Conversation
|
EWS run on previous version of this PR (hash a4a635a) Details |
a4a635a to
511477d
Compare
|
EWS run on previous version of this PR (hash 511477d) Details |
| } | ||
| if (!m_dateTimePicker) | ||
| return; | ||
| auto rootFrameID = params.rootFrameID; |
There was a problem hiding this comment.
This local variable is unnecessary, we can read off of params in the lambda.
| return; | ||
|
|
||
| params.anchorRectInRootView = IntRect(*convertedRect); | ||
| protectedThis->m_dateTimePicker->setFrameID(rootFrameID); |
There was a problem hiding this comment.
Can we remove this setter and fold the logic into showDateTimePicker? We could read off of params inside that method.
| void WebPageProxy::didEndDateTimePicker() | ||
| { | ||
| m_dateTimePicker = nullptr; | ||
| RefPtr picker = std::exchange(m_dateTimePicker, nullptr); |
There was a problem hiding this comment.
Maybe just pull out the frameID rather than keeping the picker alive longer?
511477d to
8205bdb
Compare
|
EWS run on previous version of this PR (hash 8205bdb) Details |
8205bdb to
b60a47a
Compare
|
EWS run on previous version of this PR (hash b60a47a) Details |
|
|
||
| void WebDateTimePickerGtk::showDateTimePicker(WebCore::DateTimeChooserParameters&& params) | ||
| { | ||
| m_frameID = params.rootFrameID; |
There was a problem hiding this comment.
It would be nice to use the same platformShowDateTimePicker pattern I suggested in the other PR.
b60a47a to
781c669
Compare
|
EWS run on current version of this PR (hash 781c669) Details |
https://bugs.webkit.org/show_bug.cgi?id=308921 rdar://171461287 Reviewed by Aditya Keerthi. The anchorRectInRootView in DateTimeChooserParameters is relative to the iframe's root view, but the UI process interprets it as main-frame coordinates. Add rootFrameID to DateTimeChooserParameters and use convertRectToMainFrameCoordinates() in showDateTimePicker, matching the existing color picker fix. Also store the frameID on WebDateTimePicker so didChooseDate/didEndDateTimePicker route responses reliably instead of using focusedOrMainFrame(). * Source/WebCore/html/BaseDateAndTimeInputType.cpp: (WebCore::BaseDateAndTimeInputType::setupDateTimeChooserParameters): * Source/WebCore/platform/DateTimeChooserParameters.h: * Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in: * Source/WebKit/UIProcess/WebDateTimePicker.cpp: (WebKit::WebDateTimePicker::showDateTimePicker): * Source/WebKit/UIProcess/WebDateTimePicker.h: (WebKit::WebDateTimePicker::frameID const): * Source/WebKit/UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::showDateTimePicker): (WebKit::WebPageProxy::didChooseDate): (WebKit::WebPageProxy::didEndDateTimePicker): * Source/WebKit/UIProcess/gtk/WebDateTimePickerGtk.cpp: (WebKit::WebDateTimePickerGtk::platformShowDateTimePicker): (WebKit::WebDateTimePickerGtk::showDateTimePicker): * Source/WebKit/UIProcess/gtk/WebDateTimePickerGtk.h: * Source/WebKit/UIProcess/mac/WebDateTimePickerMac.h: * Source/WebKit/UIProcess/mac/WebDateTimePickerMac.mm: (WebKit::WebDateTimePickerMac::platformShowDateTimePicker): (WebKit::WebDateTimePickerMac::showDateTimePicker): Canonical link: https://commits.webkit.org/309104@main
781c669 to
8263290
Compare
|
Committed 309104@main (8263290): https://commits.webkit.org/309104@main Reviewed commits have been landed. Closing PR #59667 and removing active labels. |
🛠 ios-apple
8263290
781c669
🛠 win🧪 win-tests🧪 api-mac-debug