Closed
Bug 1076967
Opened 10 years ago
Closed 10 years ago
Fix Error object data propagation to Loop content pages
Categories
(Hello (Loop) :: Client, defect)
Hello (Loop)
Client
Tracking
(firefox33 wontfix, firefox34+ fixed, firefox35+ fixed, firefox36 fixed)
People
(Reporter: mikedeboer, Assigned: mikedeboer)
Details
Attachments
(1 file, 1 obsolete file)
1.35 KB,
patch
|
bholley
:
review+
lmandel
:
approval-mozilla-aurora+
lmandel
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
I noticed earlier that Error object didn't contain the values I expected, so I asked bholley on IRC what could be the cause... a lengthy conversation followed and the end-result is this patch.
Attachment #8498974 -
Flags: review?(bobbyholley)
Flags: qe-verify-
Flags: needinfo?(mmucci)
Flags: firefox-backlog+
Comment 2•10 years ago
|
||
Comment on attachment 8498974 [details] [diff] [review] Patch Review of attachment 8498974 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/loop/MozLoopAPI.jsm @@ +45,5 @@ > for (let prop of Object.getOwnPropertyNames(error)) { > + Object.defineProperty(Cu.waiveXrays(obj), prop, { > + configurable: false, > + enumerable: true, > + value: String(error[prop]) This is wrong for things like line number, right?
Attachment #8498974 -
Flags: review?(bobbyholley) → review-
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Bobby Holley (:bholley) from comment #2) > This is wrong for things like line number, right? Why would that be wrong? The goal is to move the Error object past the chrome->content barrier as-is. The Error object from the targetWindow has its own lineNumber and fileName properties that'll always be the same for each error we pass, obscuring useful information from the original error object we're trying to clone, IF we don't overwrite these properties too.
Flags: needinfo?(bobbyholley)
Comment 4•10 years ago
|
||
(In reply to Mike de Boer [:mikedeboer] from comment #3) > (In reply to Bobby Holley (:bholley) from comment #2) > > This is wrong for things like line number, right? > > Why would that be wrong? Because you're stringifying an integer, so you'll end up with |"42"| instead of |42|.
Flags: needinfo?(bobbyholley)
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8498974 -
Attachment is obsolete: true
Attachment #8504638 -
Flags: review?(bobbyholley)
Updated•10 years ago
|
Attachment #8504638 -
Flags: review?(bobbyholley) → review+
Assignee | ||
Comment 6•10 years ago
|
||
Thanks, Bobby! Pushed to fx-team as https://hg.mozilla.org/integration/fx-team/rev/14cfea6cd6ae
Updated•10 years ago
|
Iteration: 35.3 → 36.1
https://hg.mozilla.org/mozilla-central/rev/14cfea6cd6ae
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Comment 8•10 years ago
|
||
Comment on attachment 8504638 [details] [diff] [review] Patch v1.1: fix Error object data propagation to Loop content pages Approval Request Comment [Feature/regressing bug #]: Hello contact errors [User impact if declined]: If there is a problem with Hello contacts, the user won't see any error messages in the browser console [Describe test coverage new/current, TBPL]: landed in m-c [Risks and why]: low risk to anything outside of Hello. Without this the user has no way to know why a problem occurred [String/UUID change made/needed]: No strings
Attachment #8504638 -
Flags: approval-mozilla-beta?
Attachment #8504638 -
Flags: approval-mozilla-aurora?
Comment 9•10 years ago
|
||
[Tracking Requested - why for this release]: Requesting uplift - -See Comment 8
tracking-firefox34:
--- → ?
Updated•10 years ago
|
status-firefox33:
--- → wontfix
status-firefox34:
--- → affected
status-firefox35:
--- → affected
status-firefox36:
--- → fixed
tracking-firefox35:
--- → +
Comment 10•10 years ago
|
||
Tested on Nightly 10/17 (so I could generate an error trying to import contacts) and works.
Comment 11•10 years ago
|
||
Comment on attachment 8504638 [details] [diff] [review] Patch v1.1: fix Error object data propagation to Loop content pages Thanks for testing jesup. Approved for Aurora. If everything goes well with your testing on Sunday, we'll get this uplifted for beta2.
Attachment #8504638 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 13•10 years ago
|
||
Tested and good on Aurora nightly build; windows and linux.
Comment 15•10 years ago
|
||
Comment on attachment 8504638 [details] [diff] [review] Patch v1.1: fix Error object data propagation to Loop content pages Previously approved offline. Adding approval to the bug.
Attachment #8504638 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in
before you can comment on or make changes to this bug.
Description
•