Closed Bug 1144571 Opened 10 years ago Closed 10 years ago

First bookmark is the last one (Cut - Paste).

Categories

(Toolkit :: Places, defect)

Product:
Toolkit
Component:
Places
Version:
35 Branch
Platform:
x86
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal
Points:
3

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla39
Iteration:
39.3 - 30 Mar
Tracking Flags:
Tracking Status
firefox36 --- wontfix
firefox37 --- wontfix
firefox38 --- fixed
firefox39 --- fixed

People

(Reporter: look997, Assigned: mak)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

patch v1.1
(deleted), patch
ttaubert
: review+
Sylvestre
: approval-mozilla-aurora+
Details | Diff | Splinter Review
User Agent: Mozilla/5.0 (Windows NT 6.1; rv:36.0) Gecko/20100101 Firefox/36.0 Build ID: 20150305021524 Steps to reproduce: I cut several bookmarks. Paste the bookmark to another empty folder. Actual results: First bookmark is the last one. The rest is in the normal order. Before: Folder1 -bookmark1 -bookmark2 -bookmark3 -bookmark4 Folder2[empty] After: Folder1[empty] Folder2 -bookmark2 -bookmark3 -bookmark4 -bookmark1 Expected results: First bookmark is the first one. The rest is in the normal order. Before: Folder1 -bookmark1 -bookmark2 -bookmark3 -bookmark4 Folder2[empty] After: Folder1[empty] Folder2 -bookmark1 -bookmark2 -bookmark3 -bookmark4
Component: Untriaged → Bookmarks & History
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b00bdb144e06&tochange=a084c4cfd8a1 Suspect: 6ef192784187 Asaf Romano — Bug 1067954 - Async Places Transactions: Paste functionality. r=mak
Blocks: 1067954
Status: UNCONFIRMED → NEW
Component: Bookmarks & History → Places
Ever confirmed: true
Keywords: regression
Product: Firefox → Toolkit
Version: 36 Branch → 35 Branch
Mano could you please verify this is working properly in the new PT code?
Flags: needinfo?(mano)
I think the problem in the legacy code is: let insertionIndex = ip.index + i; should be let insertionIndex = ip.index; But I'm not sure why this was changed and why it differs from the new code. We clearly need a test here.
Points: --- → 3
Flags: qe-verify-
Flags: in-testsuite?
Flags: firefox-backlog+
I'm taking this regression fix
Assignee: nobody → mak77
Flags: needinfo?(mano)
Status: NEW → ASSIGNED
Iteration: --- → 39.2 - 23 Mar
Iteration: 39.2 - 23 Mar → 39.3 - 30 Mar
Attached patch patch v1 (obsolete) (deleted) — — Splinter Review
This was an unintended change, the test will ensure we don't regress it again.
Attachment #8584559 - Flags: review?(ttaubert)
Attached patch patch v1.1 (deleted) — — Splinter Review
oops, the timeouts were not expected to be there.
Attachment #8584559 - Attachment is obsolete: true
Attachment #8584559 - Flags: review?(ttaubert)
Attachment #8584560 - Flags: review?(ttaubert)
Attachment #8584560 - Flags: review?(ttaubert) → review+
https://hg.mozilla.org/integration/fx-team/rev/b7a9b5a6983b
Flags: in-testsuite? → in-testsuite+
Target Milestone: --- → mozilla39
Comment on attachment 8584560 [details] [diff] [review] patch v1.1 Approval Request Comment [Feature/regressing bug #]: bug 1067954 [User impact if declined]: pasting multiple bookmarks messes up their order [Describe test coverage new/current, TreeHerder]: automated test [Risks and why]: very low risk, restoring code as it was before unintended change [String/UUID change made/needed]: none
Attachment #8584560 - Flags: approval-mozilla-aurora?
Note, the approval is for FF38, if we merge before the approval, it will be beta, not aurora.
Attachment #8584560 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: