Closed
Bug 835612
Opened 11 years ago
Closed 11 years ago
Fix nsVolumeService to use strings better
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: dhylands, Assigned: baku)
References
Details
Attachments
(1 file)
2.68 KB,
patch
|
justin.lebar+bug
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #835408 +++ In particular bug 835408 comment 7: > nsString stateStr(NS_ConvertUTF8toUTF16(vol->StateStr())); > obs->NotifyObservers(vol, NS_VOLUME_STATE_CHANGED, stateStr.get()); On an unrelated note, I believe NS_ConvertUTF8toUTF16 derives from nsAutoString, so if the conversion results in a short enough string it gets stored in the auto buffer and then promptly copied to stateStr. There are two nicer ways to write this: NS_ConvertUTF8toUTF16 stateStr(vol->StateStr()); obs->NotifyObservers(vol, NS_VOLUME_STATE_CHANGED, stateStr.get()); or obs->NotifyObservers(vol, NS_VOLUME_STATE_CHANGED, NS_ConvertUTF8toUTF16(vol->StateStr()).get());
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #707580 -
Flags: review?(justin.lebar+bug)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → amarchesini
Comment 2•11 years ago
|
||
Comment on attachment 707580 [details] [diff] [review] patch Sure. You could even do NotifyObservers(..., NS_ConvertUTF8toUTF16(volume->StateStr()).get()); if you like. Either way is fine with me.
Attachment #707580 -
Flags: review?(justin.lebar+bug) → review+
Assignee | ||
Comment 4•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b03b419446f
Comment 5•11 years ago
|
||
(In reply to Andrea Marchesini (:baku) from comment #3) > Should this patch land to b2g18? Does this patch solve some immediate problem?
Assignee | ||
Comment 6•11 years ago
|
||
Not really :) It's just a small optimization.
Comment 7•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b03b419446f Please post changeset links and remove checkin-needed when landing your patches on inbound so others don't waste their time attempting to do the same.
Keywords: checkin-needed
Assignee | ||
Comment 8•11 years ago
|
||
I forgot to remove checkin-needed. Sorry for this.
Comment 9•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7b03b419446f
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•