Closed
Bug 870856
Opened 11 years ago
Closed 11 years ago
Convert DOMError to WebIDL
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla24
People
(Reporter: baku, Assigned: baku)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-needed)
Attachments
(1 file, 3 obsolete files)
53.23 KB,
patch
|
baku
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•11 years ago
|
||
Still waiting for try. Probably it breaks something, but a first round of review can be taken :)
Attachment #748024 -
Flags: review?(Ms2ger)
Comment 2•11 years ago
|
||
Comment on attachment 748024 [details] [diff] [review] patch Review of attachment 748024 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, but I'd like to see a version that passes try ::: dom/apps/src/Webapps.js @@ +428,5 @@ > return this._ondownloadapplied; > }, > > get downloadError() { > + return DOMError(this._downloadError); You tested this? ::: dom/indexedDB/IDBRequest.cpp @@ +162,5 @@ > NS_ASSERTION(NS_FAILED(aRv), "Er, what?"); > NS_ASSERTION(!mError, "Already have an error?"); > > mHaveResultOrErrorCode = true; > + mError = new DOMError(mTransaction->GetOwner(), aRv); Looks like this can be null ::: dom/indexedDB/IDBTransaction.cpp @@ +593,1 @@ > return AbortInternal(aRequest->GetErrorCode(), error.forget()); Add a DOMError* IDBRequest::GetError(ErrorResult& aRv) and use it here. ::: dom/indexedDB/IndexedDatabaseManager.cpp @@ +203,3 @@ > NS_ENSURE_SUCCESS(rv, rv); > > + nsRefPtr<DOMError> error = static_cast<DOMError*>(ptr.get()); Ditto ::: dom/indexedDB/OpenDatabaseHelper.cpp @@ +2413,3 @@ > DebugOnly<nsresult> rv = > + mOpenDBRequest->GetError(getter_AddRefs(ptr)); > + nsRefPtr<DOMError> error = static_cast<DOMError*>(ptr.get()); Ditto ::: dom/telephony/TelephonyCall.h @@ +21,5 @@ > nsRefPtr<Telephony> mTelephony; > > nsString mNumber; > nsString mState; > + nsRefPtr<mozilla::dom::DOMError> mError; Probably needs an include. ::: dom/webidl/DOMError.webidl @@ +5,3 @@ > * > * The origin of this IDL file is > * http://www.w3.org/TR/2012/WD-dom-20120105/ Fix the URL
Attachment #748024 -
Flags: review?(Ms2ger)
Updated•11 years ago
|
Keywords: dev-doc-needed
Comment 3•11 years ago
|
||
Comment on attachment 748024 [details] [diff] [review] patch ::: dom/webidl/DOMError.webidl >@@ -5,11 +5,13 @@ > readonly attribute DOMString name; >+ readonly attribute DOMString message; [Constant]?
Assignee | ||
Comment 4•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=3385e795266f
Attachment #748024 -
Attachment is obsolete: true
Attachment #749873 -
Flags: review?(Ms2ger)
Comment 5•11 years ago
|
||
Comment on attachment 749873 [details] [diff] [review] patch Review of attachment 749873 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/base/public/nsIDOMFileReader.idl @@ +26,5 @@ > > [implicit_jscontext] > readonly attribute jsval result; > + > + // This is a DOMError Indentation ::: dom/apps/src/Webapps.js @@ +428,5 @@ > return this._ondownloadapplied; > }, > > get downloadError() { > + return new this._window.DOMError(this._downloadError ? this._downloadError : ''); this._downloadError || "" ::: dom/base/DOMError.cpp @@ +7,5 @@ > +#include "mozilla/dom/DOMError.h" > +#include "mozilla/dom/DOMErrorBinding.h" > +#include "nsDOMException.h" > +#include "nsContentUtils.h" > +#include "nsPIDOMWindow.h" Sort @@ +15,2 @@ > > +NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE_0(DOMError) Why not _1(DOMError, mWindow)? ::: dom/base/DOMError.h @@ +10,5 @@ > +#include "mozilla/Attributes.h" > +#include "mozilla/ErrorResult.h" > +#include "nsCycleCollectionParticipant.h" > +#include "mozilla/dom/BindingUtils.h" > +#include "nsWrapperCache.h" Sort
Attachment #749873 -
Flags: review?(Ms2ger) → review+
Assignee | ||
Comment 6•11 years ago
|
||
Attachment #749873 -
Attachment is obsolete: true
Attachment #750968 -
Flags: review+
Assignee | ||
Updated•11 years ago
|
Attachment #750968 -
Flags: review?(bzbarsky)
Comment 7•11 years ago
|
||
Comment on attachment 750968 [details] [diff] [review] patch May be worth documenting in the header that the window passed to the constructors may be null if the DOMError is not associated with a particular window. You should rev the IIDs of the various xpidl interfaces you change here. This is very nice! r=me
Attachment #750968 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 8•11 years ago
|
||
Attachment #750968 -
Attachment is obsolete: true
Attachment #751311 -
Flags: review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 9•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1196b497640b
Flags: in-testsuite?
Keywords: checkin-needed
Assignee | ||
Updated•11 years ago
|
Flags: in-testsuite? → in-testsuite+
Comment 10•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1196b497640b
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•