793031 - Open recent file fails if the file doesn't exists anymore

Status: RESOLVED FIXED

(DevTools Graveyard :: Scratchpad, defect, P2)

Description

[Andres Hernandez [:andreshm]]

The issue is when you open a file using the File > Open Recent menu, but the file doesn't exists anymore. 

Currently it shows two selected items in the list and there is an error in console: 

JavaScript error: resource://gre/modules/NetUtil.jsm, line 165: NS_ERROR_FILE_NOT_FOUND: Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIChannel.asyncOpen]

I suggest to open the file picker and remove/update the entry on the recent list.

Comment 1

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

Hi Andres. Thanks for the bug report. Good catch.

Comment 2

[Johan C]

Attachment: patch 1 wip

WIP patch:

I'm using the nofication bar to tell the user if a file doesn't exist.

Thoughts?

Comment 3

[Johan C]

Attachment: Screenshot 1 of patch 1

Comment 4

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

Comment on attachment 663543 [details] [diff] [review]
patch 1 wip

should at least clear the entry from the recent files list too.

Comment 5

[Johan C]

oh indeed

Comment 6

[Johan C]

Attachment: patch 2 wip

Introduced a new method for clearing one or more files.

* Clears the file from the menu.

Comment 7

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

Comment on attachment 663545 [details]
Screenshot 1 of patch 1

The language feels a little funny here. Mostly the "you" makes it sound like we're having a conversation with the user.

How about, "This file no longer exists."?

Comment 8

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

Comment on attachment 663571 [details] [diff] [review]
patch 2 wip

looks good.

Thought about a test yet?

Comment 9

[Johan C]

Attachment: patch 3

Changes in this version:
1. Changed the name of the l10n-string from 'fileExists.notification' to 'fileNoLongerExists.notification'.

2. Added tests.

3. Changed the notification value from 'SCRATCHPAD_CONTEXT_BROWSER' to "file-no-longer-exists".

4. Changed the notification label per your feedback.

5. Rebased 2012-10-01.

Cheers!

Comment 10

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

Comment on attachment 666664 [details] [diff] [review]
patch 3

looks good! We should send this through the try servers for our amusement.

Comment 11

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

sorry for the delay, Johan.

Try results here:

https://tbpl.mozilla.org/?tree=Try&rev=a8a9a50ce32e

Comment 12

[Panos Astithas (he/him) [:past] (please ni?)]

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

Comment 13

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/fd093acb629e

Comment 14

[Ed Morley [:emorley]]

Unfortunately I had to back this out for causing bug 801660.

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7703f0ec6770

Comment 15

[Ed Morley [:emorley]]

(In reply to Ed Morley [:edmorley UTC+1] from comment #14)
> Unfortunately I had to back this out for causing bug 801660.
> 
> Backout:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/7703f0ec6770

https://hg.mozilla.org/mozilla-central/rev/7703f0ec6770

Comment 16

[Johan C]

Attachment: patch 4

Thank you Ed :)

This patch should hopefully fix the intermittent oranges.

Rob, could you take another look?

Changes from previous patch:
1. The test now removes 'gFile04' rather than 'gFile02'. 'gFile02' was the active file in Scratchpad and might have been what caused the failures.

2. Added the function 'waitForFileDeletion()'. This should make sure the file ('gFile04') has been removed before proceeding with the test.

Cheers!

Comment 17

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

Comment on attachment 672046 [details] [diff] [review]
patch 4

+function waitForFileDeletion() {
+  if (gFile04.exists()) {
+    setTimeout(waitForFileDeletion, 0);

you can use executeSoon() instead of setTimeout() here. Otherwise looks good. I'll send it through the try gauntlet.

Comment 18

[Johan C]

Attachment: patch 4.1

* Use executeSoon() instead of setTimeout().

Carrying forward r+ from 'patch 4'.

Comment 19

[Johan C]

Attachment: patch 4.2

Rebased,

cheers!

Comment 20

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

Comment on attachment 673877 [details] [diff] [review]
patch 4.2

this should work

Comment 21

[Panos Astithas (he/him) [:past] (please ni?)]

Johan, this patch needs rebasing.

Comment 22

[Johan C]

Attachment: patch 4.3

rebased

Comment 23

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

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

\o/

Thanks, Johan!

Comment 24

[Ed Morley [:emorley]]

This landed on m-c some time ago, but the bugs were not updated:
https://hg.mozilla.org/mozilla-central/rev/fd245024ffad