Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Warn for showModalDialog uses

RESOLVED FIXED in mozilla28

Status

()

Core
DOM
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: sicking, Assigned: vendo)

Tracking

({dev-doc-complete, site-compat})

unspecified
mozilla28
dev-doc-complete, site-compat
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=jdm][lang=c++])

Attachments

(1 attachment, 2 obsolete attachments)

showModalDialog is a terrible API, both to implement and to use. And it's relatively little used on the web.

We should try to remove the last remaining usage by warning in the developer console whenever it is used. Chrome just did the same so we're not going alone.
This should be trivial, we just need to call ReportUseOfDeprecatedMethod() in nsGlobalWindow::ShowModalDialog() here, and set the message up for localization etc.
Whiteboard: good first bug

Updated

4 years ago
Whiteboard: good first bug → [good first bug][mentor=jdm][lang=c++]
We also have WarnOnceAbout() which appear to have the same purpose as ReportUseOfDeprecatedMethod(). I filed bug 933563 on consolidating them :)

Comment 3

4 years ago
I would like to work on it.
Flags: needinfo?(nobody)

Updated

4 years ago
Assignee: nobody → riteshnpatel1994
Flags: needinfo?(nobody)
(Assignee)

Comment 4

4 years ago
Created attachment 826031 [details] [diff] [review]
bug933040

patch
(Assignee)

Comment 5

4 years ago
ups I am sorry Ritesh. I have done it meanwhile.
(Assignee)

Updated

4 years ago
Attachment #826031 - Flags: review?(josh)

Updated

4 years ago
Assignee: riteshnpatel1994 → vendo.ruzicka

Updated

4 years ago
Attachment #826031 - Flags: review?(josh) → review?(jonas)
Comment on attachment 826031 [details] [diff] [review]
bug933040

Stealing review, and thanks vendo! r=jst
Attachment #826031 - Flags: review?(jonas) → review+
Comment on attachment 826031 [details] [diff] [review]
bug933040

>+# LOCALIZATION NOTE: Do not translate "Window.showModalDialog()"
>+ShowModalDialogWarning=Use of Window.showModalDialog() is deprecated. There is no short answer what to use instead, basically all JavaScript frameworks provide an alternative. For more help https://developer.mozilla.org/en-US/docs/Web/API/Window.showModalDialog

"Window" should be lower cased (except inside the URL).
(Assignee)

Comment 8

4 years ago
Created attachment 826265 [details] [diff] [review]
bug933040

lower case applied
Attachment #826031 - Attachment is obsolete: true
Attachment #826265 - Flags: review?(jst)

Updated

4 years ago
Attachment #826265 - Flags: review?(jst) → review+
Comment on attachment 826031 [details] [diff] [review]
bug933040

I'd say simply refer to window.open instead. showModalDialog() doesn't provide much value over what window.open does, other than that it's synchronous, and the synchronous part is not something that any frameworks can help with.
(Assignee)

Updated

4 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 10

4 years ago
Created attachment 826925 [details] [diff] [review]
bug933040

ok, patch updated
Attachment #826265 - Attachment is obsolete: true
Attachment #826925 - Flags: review?(jonas)
Attachment #826925 - Flags: review?(jonas) → review+
FWIW, I also filed bug 933042 on making WarnOnceAbout collect telemetry so that we know if/when it's safe to remove the implementation.

Comment 12

4 years ago
I guess this patch is ready to be checked in then.
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/e495c0d3c240
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e495c0d3c240
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
https://developer.mozilla.org/en-US/docs/Web/API/Window.showModalDialog
https://developer.mozilla.org/en-US/Firefox/Releases/28/Site_Compatibility
Keywords: dev-doc-complete, site-compat
You need to log in before you can comment on or make changes to this bug.