Closed
Bug 1056649
Opened 10 years ago
Closed 10 years ago
Port |Bug 559505 - localstore.rdf kills ponies| to Thunderbird
Categories
(Thunderbird :: Build Config, defect)
Thunderbird
Build Config
Tracking
(thunderbird34?, thunderbird36 fixed)
RESOLVED
FIXED
Thunderbird 36.0
People
(Reporter: sgautherie, Assigned: mkmelin)
References
Details
Attachments
(2 files)
22.16 KB,
patch
|
mconley
:
review+
Paenglab
:
review+
|
Details | Diff | Splinter Review |
1.26 KB,
patch
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #1056635 +++ (In reply to Ryan VanderMeulen [:RyanVM UTC-4] from bug 559505 comment #74) > https://hg.mozilla.org/mozilla-central/rev/25c918c5f3e1 1) Trivial config part is to copy 'browser/installer/package-manifest.in' changes. 2) Probably port some of 'browser/components/*' changes. i.e. http://mxr.mozilla.org/comm-central/search?string=rdf%3Alocal-store&case=1&find=%2Fmail%2F 3) Check to see if anything else needs to be done too.
Reporter | ||
Comment 1•10 years ago
|
||
(In reply to Serge Gautherie (:sgautherie) from comment #0) > 2) Probably port some of 'browser/components/*' changes. > i.e. > http://mxr.mozilla.org/comm-central/search?string=rdf%3Alocal- > store&case=1&find=%2Fmail%2F http://mxr.mozilla.org/comm-central/search?string=rdf%3Alocal-store&case=1&find=%2Fmail {{ Found 3 matching lines in 3 files /mail/base/modules/mailMigrator.js * line 114 -- this._dataSource = this._rdf.GetDataSource("rdf:local-store"); /mail/test/mozmill/migration-to-rdf-ui-2/test-migrate-to-rdf-ui-2.js * line 35 -- let datasource = rdf.GetDataSource("rdf:local-store"); /mailnews/base/prefs/content/AccountManager.js * line 1468 -- this._rdfDataSource = this._rdf.GetDataSource("rdf:local-store"); }}
Comment 2•10 years ago
|
||
The patch for Bug 559505 causes some functionality to break in Thunderbird: see https://bugzilla.mozilla.org/show_bug.cgi?id=559505#c77
Reporter | ||
Comment 3•10 years ago
|
||
(In reply to Roberto Agostino Vitillo (:rvitillo) from comment #2) > The patch for Bug 559505 causes some functionality to break in Thunderbird: > see https://bugzilla.mozilla.org/show_bug.cgi?id=559505#c77 (In reply to Thomas D. from bug 559505 comment #77) > > - In 08-21, several things are broken at once: the thread summary no longer > > works, tabs are not saved, the state of the folder pane (unified vs. all vs. > > ...) is not saved either. Do you have any hints as to what bugs could be > > causing this? Yes, that breakage is very likely (due to) this bug. (I'm not sure whether/how_much bug 1056948 will help by itself.)
Reporter | ||
Updated•10 years ago
|
Severity: normal → major
tracking-thunderbird34:
--- → ?
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Updated•10 years ago
|
QA Contact: mkmelin+mozilla
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8481961 -
Flags: review?(mconley)
Assignee | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8481961 [details] [diff] [review] bug1056649_tb_localstore_rdf.patch Richard, maybe you'd like to take this review? ;)
Attachment #8481961 -
Flags: review?(richard.marti)
Comment 8•10 years ago
|
||
Comment on attachment 8481961 [details] [diff] [review] bug1056649_tb_localstore_rdf.patch Review of attachment 8481961 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me with my limited knowledge. My mailMigrator.js patch in dupe bug looked the same except your patch is more elegant :). r+ with the comments addressed. ::: mail/base/modules/mailMigrator.js @@ +132,5 @@ > // We want to remove the throbber from the menubar on Linux and > // Windows, and from the mail-toolbar on OSX. > + let mailBarId = (Services.appinfo.OS == "Darwin") ? > + "mail-bar3" : "mail-toolbar-menubar2"; > + let cs = xulStore.getValue(MESSENGER_DOCURL, mailBarId, "currentset"); Here and in UI version 3 you are using cs. @@ +166,5 @@ > } > > // In UI version 4, we add the chat button to the mail toolbar. > if (currentUIVersion < 4) { > + let currentset = xulStore.getValue(MESSENGER_DOCURL, "mail-bar3", "currentset"); And here (and also in UI version 5) currentset. Wouldn't it be more consistent to use always the same variable name to make it easier for future changes? @@ +224,5 @@ > } catch(e) { > Cu.reportError("Migrating from UI version " + currentUIVersion + " to " > + UI_VERSION + " failed. Error message was: " + e + " -- " > + "Will reattempt on next start."); > + } Nit: trailing white space.
Attachment #8481961 -
Flags: review?(richard.marti) → review+
Reporter | ||
Comment 9•10 years ago
|
||
Comment on attachment 8481961 [details] [diff] [review] bug1056649_tb_localstore_rdf.patch Review of attachment 8481961 [details] [diff] [review]: ----------------------------------------------------------------- AccountManager.js already has a (new) patch in bug 1058873: maybe leave it out in the current bug.
Comment 10•10 years ago
|
||
Comment on attachment 8481961 [details] [diff] [review] bug1056649_tb_localstore_rdf.patch Review of attachment 8481961 [details] [diff] [review]: ----------------------------------------------------------------- Woo! Glad to get rid of localstore.rdf. :) Thanks Magnus! ::: mail/base/modules/mailMigrator.js @@ +166,5 @@ > } > > // In UI version 4, we add the chat button to the mail toolbar. > if (currentUIVersion < 4) { > + let currentset = xulStore.getValue(MESSENGER_DOCURL, "mail-bar3", "currentset"); I agree - consistency would be better. ::: mail/test/mozmill/migration-to-rdf-ui-2/test-migrate-to-rdf-ui-2.js @@ +40,5 @@ > > /** > * Test that the "width" property of the folderPaneBox resource was persisted. > * We do this simply be checking that the width of the folderPaneBox matches > + * the width defined in localstore.rdf (->XULStore.json) (which, in this case, is 500px). I think we should make this comment more clear about what ->XULStore.json means. I think just a quick remark that localstore.rdf has since converted to using XULStore.json was made possible by bug 559505 is probably sufficient.
Attachment #8481961 -
Flags: review?(mconley) → review+
Assignee | ||
Comment 11•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/8d1c850ff12b -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 36.0
Assignee | ||
Comment 12•10 years ago
|
||
Will push this logic error that the tests caught
Assignee | ||
Comment 13•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/cfb70d10971f
Updated•9 years ago
|
status-thunderbird36:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•