1667713 - Clean up nsresult values

Status: RESOLVED WONTFIX

(Core :: XPCOM, task)

Description

[Nicholas Nethercote [inactive]]

The existence of any success codes other than NS_OK annoys me. Some can easily be removed.

We also have some duplicate constants, some of which appear intentional, and some which do not.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Bug 1667713 - Remove NS_SUCCESS_FILE_DIRECTORY_EMPTY. r=nika

It has a single use that can be replaced with NS_OK without any other changes.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: Bug 1667713 - Remove NS_SUCCESS_EDITOR_FOUND_TARGET. r=m_kato

It's unused.

Depends on D91548

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: Bug 1667713 - Remove NS_SUCCESS_EDITOR_ELEMENT_NOT_FOUND. r=m_kato

Its uses can be replaced with NS_OK without any other changes.

Depends on D91549

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: Bug 1667713 - Remove NS_SUCCESS_RESTART_APP. r=nika

Both constants have the same value! They can be replaced by NS_OK without
effect.

Depends on D91550

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: Bug 1667713 - Remove the RDF nsresult module. r=nika

It has four constants, three of which are unused.

The one used constant is NS_RDF_NO_VALUE, which sounds like a failure code but
is actually a success code. It's only used in one place, on a failure path. But
the function that uses it has a single call site, and the return value is not
checked, which means that the NS_RDF_NO_VALUE use is never checked.

I have replaced this one use of NS_RDF_NO_VALUE with NS_ERROR_FAILURE rather
than NS_OK, because that makes more sense for a failure path.

Depends on D91551

Comment 6

[Nicholas Nethercote [inactive]]

Attachment: Bug 1667713 - Remove NS_BINDING_SUCCEEDED. r=valentin

It's a low-value synonym of NS_OK that isn't used in mozilla-central.

Depends on D91552

Comment 7

[Nicholas Nethercote [inactive]]

Attachment: Bug 1667713 - Remove NS_ERROR_HTMLPARSER_CONTINUE. r=nika

It's a low-value synonym of NS_OK that isn't used.

Depends on D91553

Comment 8

[Nicholas Nethercote [inactive]]

Attachment: Bug 1667713 - Remove NS_ERROR_XPATH_INVALID_ARG. r=nika

It's a low-value synonym of NS_ERROR_INVALID_ARG.

Depends on D91554

Comment 9

[Nicholas Nethercote [inactive]]

Attachment: Bug 1667713 - Remove NS_ERROR_ILLEGAL_INPUT. r=nika

It's a deprecated and potentially confusing synonym of
NS_ERROR_UDEC_ILLEGALINPUT.

Depends on D91555

Comment 10

[Nicholas Nethercote [inactive]]

Attachment: Bug 1667713 - Add a duplicate check to ErrorList.py. r=nika

I have included an allow-list because there are some existing dups. Some look
reasonable, some don't.

Depends on D91556

Comment 11

[Nicholas Nethercote [inactive]]

Attachment: Bug 1667713 - Remove NS_POSITION_BEFORE_TABLE. r=heycam

It's never checked for, so NS_OK is good enough.

The commit also fixes the typo in GetNextPrevLineFromeBlockFrame().

Depends on D91705

Comment 12

[Nicholas Nethercote [inactive]]

Attachment: Bug 1667713 - Rename NS_ERROR_UENC_NOMAPPING. r=nika

As NS_OK_UENC_NPMAPPING, because it's a success code!

Depends on D91706

Comment 13

[Nicholas Nethercote [inactive]]

Attachment: Bug 1667713 - Remove NS_REFRESHURI_HEADER_FOUND. r=nika

It's a low-value synonym for NS_OK.

Depends on D91707

Comment 14

[Nicholas Nethercote [inactive]]

Attachment: Bug 1667713 - Remove nsIRefreshURI::setupRefreshURI. r=nika

It's unused.

Depends on D91708

Comment 15

[Nicholas Nethercote [inactive]]

These changes ended up being more subtle than expected, with a higher chance of introducing regressions that I originally thought. (E.g. because some success codes have no explicit uses, but do have implicit uses.) Combine that with the fact that I was only able to remove some of the non-NS_OK success codes, rather than all of them as I originally hoped... I think the cautious thing is to give up on these.