Closed
Bug 1097222
Opened 10 years ago
Closed 10 years ago
Extend desktop client record format
Categories
(Firefox :: Sync, defect)
Firefox
Sync
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: rnewman, Assigned: rnewman)
References
Details
Attachments
(1 file, 1 obsolete file)
3.72 KB,
patch
|
markh
:
review+
|
Details | Diff | Splinter Review |
Desktop portion of Bug 1097218. See that bug for spec details.
Assignee | ||
Comment 1•10 years ago
|
||
To get hardware details on desktop is (obviously) OS-dependent. It's also non-trivial; on the Mac, this code #import <Foundation/Foundation.h> #include <stdlib.h> #include <stdio.h> #include <sys/types.h> #include <sys/sysctl.h> int main(int argc, const char * argv[]) { @autoreleasepool { size_t len = 0; sysctlbyname("hw.model", NULL, &len, NULL, 0); if (len) { char *model = malloc(len*sizeof(char)); sysctlbyname("hw.model", model, &len, NULL, 0); NSString *model_ns = [NSString stringWithUTF8String:model]; free(model); NSLog(@"FOO: %@", model_ns); } } return 0; } will print "MacBookPro10,1" rather than a human-readable string. We could make a network request to Apple with the serial number, or we could call system_profiler and parse the output. http://stackoverflow.com/questions/8058151/how-does-system-profiler-retrieve-the-full-mac-hardware-identifier It looks like the Windows option is: http://msdn.microsoft.com/en-us/library/windows/desktop/ms724423%28v=vs.85%29.aspx and http://stackoverflow.com/questions/1013354/how-to-check-the-machine-type-laptop-or-desktop http://blogs.technet.com/b/heyscriptingguy/archive/2004/09/21/how-can-i-determine-if-a-computer-is-a-laptop-or-a-desktop-machine.aspx
Assignee | ||
Comment 2•10 years ago
|
||
This should do the trick for now.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•10 years ago
|
||
Now with one fewer error!
Attachment #8523362 -
Flags: review?(mhammond)
Assignee | ||
Updated•10 years ago
|
Attachment #8523361 -
Attachment is obsolete: true
Comment 4•10 years ago
|
||
Comment on attachment 8523362 [details] [diff] [review] Extend client record format on desktop. v2 Review of attachment 8523362 [details] [diff] [review]: ----------------------------------------------------------------- I'd have thought it would be nice for a test to check at least there are non-empty strings for the ones we expect, but that's at your discretion. ::: services/sync/modules/engines/clients.js @@ +31,5 @@ > > +Utils.deferGetSet(ClientsRec, > + "cleartext", > + ["name", "type", "commands", > + "version", "protocols", I'd have thought you could leave this line on the previous line and and up with 2 lines roughly the same length? @@ +429,5 @@ > + record.appPackage = appInfo.ID; > + record.application = this.engine.brandName // "Nightly" > + > + // We can't compute these yet. > + // record.device = ""; FWIW, I notice that http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#3368 grabs the manufacturer for crash reporting, but yeah, it sounds hard to get this generically in the medium term.
Attachment #8523362 -
Flags: review?(mhammond) → review+
Assignee | ||
Comment 5•10 years ago
|
||
> I'd have thought it would be nice for a test to check at least there are > non-empty strings for the ones we expect, but that's at your discretion. *grumble grumble* :D Yeah, I'll write a test or two before I land. > I'd have thought you could leave this line on the previous line and and up > with 2 lines roughly the same length? This is more a semantic distinction than a line-length thing: version/protocols were the 1.5 semi-optional extension fields, with name/type/commands the original non-optional core set. I didn't care enough to bother adding piles of comments, but splitting on a notional category felt right to me. > FWIW, I notice that > http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner. > cpp#3368 grabs the manufacturer for crash reporting... Great pointer, thanks! I'll be filing followups to grab more data when we can, so this will come in handy.
Assignee | ||
Comment 6•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/0b6ef3a5b009
Comment 7•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0b6ef3a5b009
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Assignee | ||
Comment 8•10 years ago
|
||
ni me for testing and uplift consideration.
Flags: needinfo?(rnewman)
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(rnewman)
Updated•6 years ago
|
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in
before you can comment on or make changes to this bug.
Description
•