nsIRDFRemoteDataSource::Refresh() should propagate error if sync refresh not supported by channel

RESOLVED FIXED in mozilla0.9.8

Status

P3
normal
RESOLVED FIXED
18 years ago
7 months ago

People

(Reporter: waterson, Assigned: tingley)

Tracking

Trunk
mozilla0.9.8

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

1.27 KB, patch
waterson
: review+
brendan
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

18 years ago
In r1.74, I put a hack into nsRDFXMLDataSource::BlockingParse() which squelched
errors from nsIChannel::Open(). This was to simplify code in the chrome
registry, allowing the chrome registry to create a new local RDF/XML file if one
does not exist.

Instead of stuffing _all_ errors from nsIChannel::Open(), we should distinguish
between channels that can support synchronous I/O (but might not have a file to
read from yet), and those that can't.
(Reporter)

Updated

18 years ago
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla1.0
(Assignee)

Comment 1

18 years ago
Created attachment 49748 [details] [diff] [review]
patch
(Reporter)

Comment 2

18 years ago
I guess I would've done it the other way around; i.e., hope that Necko is
producing a meaningful error code, stifle that, propagate other errors. What
error code does Necko deliver if the file doesn't exist?
(Assignee)

Comment 3

18 years ago
Actually, I agree -- somehow I missed the existence of NS_ERROR_FILE_NOT_FOUND.
I'm going to verify that this is what gets returned and check for other
quashable codes as well, then redo the patch.
(Assignee)

Comment 4

18 years ago
Oh, excellent.

NS_ERROR_FILE_NOT_FOUND seems to be used fairly consistently across
nsLocalFileFoo for all Foo except "Unix".  nsLocalFileUnix returns
NS_ERROR_FAILURE when the file can't be opened.  *sigh*
tingley: please patch nsLocalFileUnix.cpp in this bug, or open a separate bug
(but that's slower, and I say fix it now).

/be

Comment 6

18 years ago
This has been fixed i beleive in the work i've done in bug 94323.
I made an effort for error retvals to produce something meaningful.

Conrad is driving that effort for the moment until i free up time to jump back in.


--pete

I say, let's anticipate pete's fine work landing by doing a conflict-free merge
of just the error result fix to nsLocalFileUnix.cpp, in a patch for this bug.
tingley, you game?

/be
(Assignee)

Comment 8

18 years ago
Brendan, I am nothing but game.  Patch coming soon.

Comment 9

18 years ago
Yes, agree get it in now. I beleive there is going to be a branch cut soon to
continue the nsIFile work on. 



--pete
(Assignee)

Updated

18 years ago
Attachment #49748 - Attachment is obsolete: true
(Assignee)

Comment 10

18 years ago
Created attachment 49802 [details] [diff] [review]
patch the 2nd
(Assignee)

Comment 11

18 years ago
I'm not currently aware of other failures that need to be masked.  Waterson?

As a side note, it's only |nsLocalFile::OpenNSPRFileDesc()| that behaves
inconsistently.  |OpenANSIFileDesc()| returns NS_ERROR_FAILURE across all
platforms.  Fixing the former appears to be a matter of calling
|NS_ErrorAccordingToNSPR()|.

(Reporter)

Comment 12

18 years ago
Comment on attachment 49802 [details] [diff] [review]
patch the 2nd

Looks good; r=waterson
Attachment #49802 - Flags: review+
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)
Target Milestone: mozilla1.0 → mozilla1.0.1
(Assignee)

Comment 14

17 years ago
Pilfering this bug in order to retarget and land.  Forgive my treachery!
Assignee: waterson → tingley
Status: ASSIGNED → NEW
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Target Milestone: mozilla1.0.1 → mozilla0.9.8
(Assignee)

Comment 16

17 years ago
fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 17

16 years ago
tever is not RDF QA anymore
QA Contact: tever → nobody

Updated

7 months ago
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.