RESOLVED WONTFIX 79027
Output a message on the console when using showModalDialog discouraging its use
https://bugs.webkit.org/show_bug.cgi?id=79027
Summary Output a message on the console when using showModalDialog discouraging its use
jochen
Reported 2012-02-20 04:39:25 PST
Output a message on the console when using showModalDialog discouraging its use
Attachments
Patch (3.21 KB, patch)
2012-02-20 04:39 PST, jochen
no flags
Patch (4.14 KB, patch)
2012-02-21 03:46 PST, jochen
no flags
Patch (4.17 KB, patch)
2012-02-24 01:48 PST, jochen
no flags
Patch for landing (5.77 KB, patch)
2012-02-24 12:29 PST, jochen
no flags
jochen
Comment 1 2012-02-20 04:39:56 PST
jochen
Comment 2 2012-02-20 04:40:45 PST
Hey Sam, what do you think about this change?
WebKit Review Bot
Comment 3 2012-02-20 06:38:18 PST
Comment on attachment 127807 [details] Patch Attachment 127807 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11552166 New failing tests: inspector/console/console-long-eval-crash.html
Adam Barth
Comment 4 2012-02-20 19:53:27 PST
Comment on attachment 127807 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=127807&action=review > Source/WebCore/page/DOMWindow.cpp:1903 > + DEFINE_STATIC_LOCAL(String, consoleMessage, ("consider using window.open instead of window.showModalDialog")); Can we say something stronger about how showModalDialog might be removed in a future version? (Also, please capitalize the "c" in Consider.)
jochen
Comment 5 2012-02-21 03:46:06 PST
Adam Barth
Comment 6 2012-02-22 15:22:15 PST
Comment on attachment 127951 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=127951&action=review > Source/WebCore/page/DOMWindow.cpp:1903 > + DEFINE_STATIC_LOCAL(String, consoleMessage, ("window.showModalDialog is broken and deprecated in WebKit. It will be removed from the engine in the future.")); Apple isn't going to like commenting on future plans. Perhaps "It will likely be removed ..." ?
WebKit Commit Bot
Comment 7 2012-02-23 19:28:54 PST
Attachment 127951 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 LayoutTests/ChangeLog:10: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
jochen
Comment 8 2012-02-24 01:48:02 PST
jochen
Comment 9 2012-02-24 12:29:33 PST
Created attachment 128782 [details] Patch for landing
Alexey Proskuryakov
Comment 10 2012-02-24 14:22:42 PST
Comment on attachment 128782 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=128782&action=review As you already mentioned on IRC, the value of these messages is questionable at best. I don't think that users of enterprise software that likes showModalDialog would be a grateful audience for warnings they can't fix (OTOH, end users don't open Web Inspector anyway). Another reason why showModalDialog would be hard to remove in foreseeable future is that Adobe installer on OS X uses it (not on the web, but from a WebView in a native application). > Source/WebCore/ChangeLog:6 > + Reviewed by Jochen Eisinger. Adam reviewed this. > Source/WebCore/page/DOMWindow.cpp:1835 > + DEFINE_STATIC_LOCAL(String, consoleMessage, ("window.showModalDialog is broken and deprecated in WebKit. It will likely be removed from the engine in the future.")); I don't think that WebKit should ever officially claim that something "is broken". Better wording is needed.
jochen
Comment 11 2012-02-24 15:00:01 PST
(In reply to comment #10) > (From update of attachment 128782 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=128782&action=review > > As you already mentioned on IRC, the value of these messages is questionable at best. Oh, I wouldn't put it like that. I think it can help to slow down the addition of new websites using this. > > I don't think that users of enterprise software that likes showModalDialog would be a grateful audience for warnings they can't fix (OTOH, end users don't open Web Inspector anyway). > > Another reason why showModalDialog would be hard to remove in foreseeable future is that Adobe installer on OS X uses it (not on the web, but from a WebView in a native application). > > > Source/WebCore/ChangeLog:6 > > + Reviewed by Jochen Eisinger. > > Adam reviewed this. ouch, good catch > > > Source/WebCore/page/DOMWindow.cpp:1835 > > + DEFINE_STATIC_LOCAL(String, consoleMessage, ("window.showModalDialog is broken and deprecated in WebKit. It will likely be removed from the engine in the future.")); > > I don't think that WebKit should ever officially claim that something "is broken". Better wording is needed. Do you have a suggestion? I took this text from UIEvent.cpp
Brian Burg
Comment 12 2014-12-17 16:36:19 PST
I don't think there is interest in pushing this feature request further.
Note You need to log in before you can comment on or make changes to this bug.