rdar://89261960
<rdar://problem/89261960>
<rdar://89261960>
Created attachment 452886 [details] Patch
Comment on attachment 452886 [details] Patch rs=me
Comment on attachment 452886 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452886&action=review > Source/WebCore/page/DOMWindow.cpp:2673 > + if (RefPtr document = this->document()) > + document->addConsoleMessage(MessageSource::JS, MessageLevel::Warning, "The showModalDialog() API is deprecated, please use the <dialog> element instead."_s); We should use an idiom more like what is used in DOMWindow::printErrorMessage. Maybe add a printWarningMessage function alongside that and use it? There’s no need to go through the document to get to the console. Ideally, would like this wording to match what we use elsewhere. Not sure we should use "()" after the function name, nor the term "API", not sure we should say "deprecated" either. Error messages in this file also say "window." in them.
(In reply to Darin Adler from comment #5) > Comment on attachment 452886 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=452886&action=review > > > Source/WebCore/page/DOMWindow.cpp:2673 > > + if (RefPtr document = this->document()) > > + document->addConsoleMessage(MessageSource::JS, MessageLevel::Warning, "The showModalDialog() API is deprecated, please use the <dialog> element instead."_s); > > We should use an idiom more like what is used in > DOMWindow::printErrorMessage. Maybe add a printWarningMessage function > alongside that and use it? There’s no need to go through the document to get > to the console. I'm doing this in bug 237047 > Ideally, would like this wording to match what we use elsewhere. Not sure we > should use "()" after the function name, nor the term "API", not sure we > should say "deprecated" either. Error messages in this file also say > "window." in them. I'll take care of this along with gardening tomorrow.
Created attachment 452939 [details] [fast-cq] Patch
Committed r290351 (247669@main): <https://commits.webkit.org/247669@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 452939 [details].
<rdar://problem/89339650>
It appears that the following three tests also need a rebase to take into account the new console warning: fast/events/beforeunload-showModalDialog.html fast/events/unload-showModalDialog.html fast/events/pagehide-showModalDialog.html HISTORY: https://results.webkit.org/?suite=layout-tests&suite=layout-tests&suite=layout-tests&test=fast%2Fevents%2Fbeforeunload-showModalDialog.html&test=fast%2Fevents%2Fpagehide-showModalDialog.html&test=fast%2Fevents%2Funload-showModalDialog.html Basically, these all started failing on Mac wk1 and Windows wk1 at r290351. So I have re-based them here: https://trac.webkit.org/changeset/290404/webkit
Reverted r290351 and r290404 for reason: Broke the build after revert of initial commit (r290348) Rebases no longer needed due to revert Committed r290406 (?): <https://commits.webkit.org/r290406>
(In reply to Robert Jenner from comment #12) > Reverted r290351 and r290404 for reason: > > Broke the build after revert of initial commit (r290348) Rebases no longer > needed due to revert > > Committed r290406 (?): <https://commits.webkit.org/r290406> r290351 needed to be reverted because it referenced 'printWarningMessage' changes that were made at r290348. That was reverted at r290399, but because r290351 referenced that, we had to revert that as well because it caused build failures. Also reverted was r290404 which were rebases for wk1 Mac and wk1 Win that needed to happen due to the new console warnings.
Created attachment 453071 [details] Patch
Committed r290412 (247720@main): <https://commits.webkit.org/247720@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 453071 [details].