Closed Bug 806656 Opened 7 years ago Closed 7 years ago

Query records generated incorrectly

Categories

(Firefox for Android :: Android Sync, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19
Tracking Status
firefox16 --- affected
firefox17 --- fixed
firefox18 --- fixed
firefox19 --- fixed

People

(Reporter: rnewman, Assigned: rnewman)

References

Details

(Whiteboard: [qa+])

Attachments

(1 file, 1 obsolete file)

We're getting records without a URI, and with a blank folderName and queryId. Android Sync is at fault.
Blocks: 806460
After the fixes in that branch, we:

* Correctly include bmkUri in produced bookmark records for query URIs.
* Omit fields which were encoded as empty strings prior to these fixes (e.g., if a query had no queryId, it now doesn't produce a blank string in the JSON output).
* No longer encode missing fields as empty strings in the first place.

This will prevent the upload of records which caused Bug 806460.
Whiteboard: [waiting for review]
Attached patch Proposed patch. v1 (obsolete) — Splinter Review
Same as the GitHub branch.
Attachment #676485 - Flags: review?(liuche)
https://hg.mozilla.org/integration/mozilla-inbound/rev/759b0e64b8b7

See Bug 806580 Comment 12 for how to verify this.

Will wait for verification before requesting uplift.
Whiteboard: [waiting for review] → [qa+]
Target Milestone: --- → mozilla19
Attached patch Aurora: fix.Splinter Review
Reviewed on GitHub. liuche, please rubberstamp here.

Setting Aurora and Beta flags here for release drivers to take a look at. Will wait for verification before landing, of course.

This is the counterpart to Bug 806460, the fix for which was just uplifted.

This is a pretty straightforward fix, and now comes with tests in the services codebase.
Attachment #676485 - Attachment is obsolete: true
Attachment #676485 - Flags: review?(liuche)
Attachment #676767 - Flags: review?(liuche)
Attachment #676767 - Flags: approval-mozilla-beta?
Attachment #676767 - Flags: approval-mozilla-aurora?
(In reply to Richard Newman [:rnewman] from comment #4)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/759b0e64b8b7
> 
> See Bug 806580 Comment 12 for how to verify this.
> 
> Will wait for verification before requesting uplift.

I believe you meant https://bugzilla.mozilla.org/show_bug.cgi?id=806460#c12. How is this the same verification?
QA Contact: twalker
(In reply to Tracy Walker [:tracy] from comment #6)
> (In reply to Richard Newman [:rnewman] from comment #4)
> > https://hg.mozilla.org/integration/mozilla-inbound/rev/759b0e64b8b7
> > 
> > See Bug 806580 Comment 12 for how to verify this.
> > 
> > Will wait for verification before requesting uplift.
> 
> I believe you meant https://bugzilla.mozilla.org/show_bug.cgi?id=806460#c12.
> How is this the same verification?

"and those queries won't appear (e.g., smart bookmarks on your toolbar won't appear). After Bug 806656 lands, those queries should sync successfully."
Comment on attachment 676767 [details] [diff] [review]
Aurora: fix.

Review of attachment 676767 [details] [diff] [review]:
-----------------------------------------------------------------

Like rnewman said, reviewed on github. Looks good here.
Attachment #676767 - Flags: review?(liuche) → review+
https://hg.mozilla.org/mozilla-central/rev/759b0e64b8b7
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Comment on attachment 676767 [details] [diff] [review]
Aurora: fix.

Approving for uplift to go out with the fix in bug 806460 so that we stop creating bad records in the first place (as explained by rnewman in IRC).
Attachment #676767 - Flags: approval-mozilla-beta?
Attachment #676767 - Flags: approval-mozilla-beta+
Attachment #676767 - Flags: approval-mozilla-aurora?
Attachment #676767 - Flags: approval-mozilla-aurora+
Product: Mozilla Services → Android Background Services
QA Contact: twalker
Product: Android Background Services → Firefox for Android
You need to log in before you can comment on or make changes to this bug.