Summary: | Output a message on the console when using showModalDialog discouraging its use | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | jochen | ||||||||||
Component: | New Bugs | Assignee: | jochen | ||||||||||
Status: | RESOLVED WONTFIX | ||||||||||||
Severity: | Normal | CC: | ap, burg, commit-queue, dglazkov, priyajeet.hora, sam, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
jochen
2012-02-20 04:39:25 PST
Created attachment 127807 [details]
Patch
Hey Sam, what do you think about this change? 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 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.) Created attachment 127951 [details]
Patch
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 ..." ? 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.
Created attachment 128688 [details]
Patch
Created attachment 128782 [details]
Patch for landing
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. (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 I don't think there is interest in pushing this feature request further. |