848666 - The ScratchPad and Style Editor doesn't close after even firefox is Closed and they become unfunctional too

Status: RESOLVED FIXED

(DevTools Graveyard :: Scratchpad, defect, P2)

Description

[Dron Rathore]

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:19.0) Gecko/20100101 Firefox/19.0
Build ID: 20130215130331

Steps to reproduce:

Step 1: Open Scratchpad or Style Editor
Step 2: Close Firefox


Actual results:

The ScratchPad and Style Editor Do not closes with the firefox(though it be a utility) but now they become nonfunctional too(instead of Save and open command on ScratchPad).


Expected results:

Either they should have closed or if not they will remain nonfunctional as Components class will not exist in there context, so the better option is Open them as a Child Instance rather being  a new instance out of Firefox.

Comment 2

[Brandon Benvie [:benvie]]

Attachment: bug-848666.patch

This patch makes the Scratchpad observe for the "browser-lastwindow-close-requested" event and then attempts to close the Scratchpad. Closing of the browser window is canceled if the use cancels closing the Scratchpad.

Comment 3

[Brandon Benvie [:benvie]]

Attachment: bug-848666.patch

Woops, here's the correct version.

Comment 4

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 810797 [details] [diff] [review]
bug-848666.patch

Review of attachment 810797 [details] [diff] [review]:
-----------------------------------------------------------------

This looks like it'd do it.

Honestly, I think I would have started this patch writing a test that proves the failure state then written this patch to verify it fixes it.

R- without a unittest.

Comment 5

[Brandon Benvie [:benvie]]

Good point, I was trying to think how to write a test to proof the affirmative (that it is closed with the browser) but tests can't close the browser. Proving a negative is feasible though!

Comment 6

[Brandon Benvie [:benvie]]

Er actually that problem still exists.

Comment 7

[Dron Rathore]

Can't we apply a force close for them as after the last-window is closed of the browser? Because they make no sense in persisting after the Browser is been closed, Scratchpad becomes nonfunctional after it also the Style Editor is editing/modifying styles for which DOM(All DOM been killed)?

Comment 8

[Brandon Benvie [:benvie]]

Well the way this patch works is that if the user cancels the close, it keeps the browser window open. This patch makes it so that there's never a Scratchpad window open if no browser windows open, which addresses your concern. I'm not sure what you mean about the Style Editor, since that lives in the toolbox and the toolbox closes when the tab it's debugging is closed, so this already isn't a problem for the Style Editor (and anything else that lives in the toolbox).

Comment 9

[Dron Rathore]

Aah my bad, Style Editor is in Dev tools now :p Okay but we can go for Scratchpad then :)

Comment 10

[Brandon Benvie [:benvie]]

Attachment: bug-848666.patch

Adds a test. https://tbpl.mozilla.org/?tree=Try&rev=82ef5d416ebf

Comment 11

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 812193 [details] [diff] [review]
bug-848666.patch

Review of attachment 812193 [details] [diff] [review]:
-----------------------------------------------------------------

woo!

Comment 12

[Rob Campbell [:rc] (:robcee)]

(woo assuming green try run of course)

Comment 13

[Rob Campbell [:rc] (:robcee)]

I guess if this fails for whatever reason, it's going to fail somewhat spectacularly with the browser closing and no more tests running. Fun!

Comment 14

[Brandon Benvie [:benvie]]

Retry since tree was broken. https://tbpl.mozilla.org/?tree=Try&rev=ed127d988b7c

Comment 15

[Brandon Benvie [:benvie]]

Different trychooser syntax. https://tbpl.mozilla.org/?tree=Try&rev=f16a4de1f33c

Comment 16

[Brandon Benvie [:benvie]]

https://hg.mozilla.org/integration/fx-team/rev/155370ed28dd

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/155370ed28dd