remove dom mochitests for showModalDialog

RESOLVED WORKSFORME

Status

()

defect
RESOLVED WORKSFORME
3 years ago
2 months ago

People

(Reporter: tracy, Unassigned)

Tracking

(Blocks 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+, firefox47 affected)

Details

(Whiteboard: btpp-active)

Attachments

(1 attachment)

Reporter

Description

3 years ago
remove test cases for showModalDialog in dom/tests/mochitest/bugs/

test_bug406375.html
test_bug437361.html
test_bug504862.html
Reporter

Comment 1

3 years ago
'hg remove' the three test files and deleted associated lines in mochitest.ini
Attachment #8725873 - Flags: review?(jmathies)
Reporter

Comment 2

3 years ago
I'll also post a patch for removing a few others. Or bundle them up all together?

dom/base/test/test_dialogArguments.html
dom/html/test/test_bug391777.html
dom/html/test/test_iframe_sandbox_modal.html
Keywords: leave-open

Updated

3 years ago
Attachment #8725873 - Flags: review?(jmathies) → review+
Reporter

Updated

3 years ago
Keywords: checkin-needed
(In reply to Tracy Walker [:tracy] from comment #2)
> I'll also post a patch for removing a few others. Or bundle them up all
> together?
> 
> dom/base/test/test_dialogArguments.html
> dom/html/test/test_bug391777.html
> dom/html/test/test_iframe_sandbox_modal.html

Are these showModalDialog related?
Why remove those? It's not dead yet in non-e10s builds, is it?
(In reply to :Ms2ger from comment #4)
> Why remove those? It's not dead yet in non-e10s builds, is it?

According to the test status spreadsheet we want these removed.

https://docs.google.com/spreadsheets/d/10UeyRoiWV2HjkWwAU51HXyXAV7YLi4BjDm55mr5Xv6c/edit?pref=2&pli=1#gid=368835050

Also, see bug 981796. Blake is busy with e10s so he probably hasn't gotten to that bug yet. Based on the comments there we could remove this from nightly today. We've given people ample warning.
Reporter

Comment 6

3 years ago
(In reply to Jim Mathies [:jimm] from comment #3) 
> > dom/base/test/test_dialogArguments.html
> > dom/html/test/test_bug391777.html
> > dom/html/test/test_iframe_sandbox_modal.html
> 
> Are these showModalDialog related?

Yes, those three are also marked "Remove" and comment showModalDialog in the test status spreadsheet.
Whiteboard: btpp-active
(In reply to Tracy Walker [:tracy] from comment #6)
> (In reply to Jim Mathies [:jimm] from comment #3) 
> > > dom/base/test/test_dialogArguments.html
> > > dom/html/test/test_bug391777.html
> > > dom/html/test/test_iframe_sandbox_modal.html
> > 
> > Are these showModalDialog related?
> 
> Yes, those three are also marked "Remove" and comment showModalDialog in the
> test status spreadsheet.

How has this decision been made? Will Firefox 47 enable e10s everywhere? Otherwise I don't think it is correct to remove tests for a shipped feature.
Or showModalDialog itself should also be removed now.
Flags: needinfo?(twalker)
Flags: needinfo?(jmathies)
Reporter

Comment 11

3 years ago
(In reply to Masatoshi Kimura [:emk] from comment #9)
> Or showModalDialog itself should also be removed now.

That is being tracked in bug 981796.
Flags: needinfo?(twalker)
Flags: needinfo?(jmathies)
Yes, so this bug should not land before bug 981796.
 (In reply to :Ms2ger from comment #12)
> Yes, so this bug should not land before bug 981796.

backed out in https://hg.mozilla.org/mozilla-central/rev/e7319545eb38 from m-c
(In reply to :Ms2ger from comment #12)
> Yes, so this bug should not land before bug 981796.

Making it explicit.
Depends on: 981796
Please don't remove DOM tests without review from a DOM peer.
Reporter

Comment 16

3 years ago
Yes, my apologies. I was under the incorrect understanding that showModalDialog was already unsupported.  I'll leave it to DOM team to remove the associated test cases when showModalDialog is disabled.
Assignee: twalker → nobody
Status: ASSIGNED → NEW
The leave-open keyword is there and there is no activity for 6 months.
:hsinyi, maybe it's time to close this bug?
Flags: needinfo?(htsai)
These tests have been removed (though I see a few related files that are still around. I'll file a bug for that.)
Flags: needinfo?(htsai)
Status: NEW → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → WORKSFORME
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.