Closed Bug 1159812 Opened 9 years ago Closed 9 years ago

"Bookmark All Tabs" is broken

Categories

(Firefox :: Bookmarks & History, defect)

40 Branch
defect
Not set
normal
Points:
3

Tracking

()

VERIFIED FIXED
Firefox 40
Iteration:
40.3 - 11 May
Tracking Status
firefox40 --- verified

People

(Reporter: alice0775, Assigned: mak)

References

Details

(Keywords: regression)

Attachments

(1 file)

[Tracking Requested - why for this release]:

Steps to reproduce
1. Right click on a tab
2. Choose "Bookmark All Tabs"
3. Expand tree view
4. Expand Bookmarks Menu
5. Select "Mozilla Firefox" folder
6. Click [Add Bookmarks] to save them

Actual Results:
When I select sub-folder, the select folder is not expanded and new folder appears under the special bookmark(such as "Bookmarks Toolbar" "Bookmarks Menu"  "Unsorted Bookmarks").
And Bookmarks are always created under the special bookmark.

Expected Results:
When I select sub-folder, the select folder should be expanded automatically and new folder should appear under the selected folder.
And [Add Bookmarks] is successfully performed


Error in Browser console:

ReferenceError: container is not defined editBookmarkOverlay.js:721:10
ReferenceError: menupopup is not defined editBookmarkOverlay.js:666:0


I wonder there are no automate tests... Too much regression due to refactoring Bug 951651.
Flags: needinfo?(mak77)
Unfortunately it's very hard to reach 100% tests coverage with such huge refactorings, we have some tests, clearly not enough, but we have few devs and moreover the tests we have need some refactoring too :/
But I'm trying to be responsive on regressions.
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Flags: needinfo?(mak77)
Points: --- → 3
Flags: qe-verify+
Flags: firefox-backlog+
Iteration: --- → 40.3 - 11 May
Attached patch patch v1Splinter Review
I'm going to file a bug to refactor browser_bookmarkProperties and add more tests to it.
Attachment #8600087 - Flags: review?(mano)
Blocks: 1160376
Attachment #8600087 - Flags: review?(ttaubert)
Blocks: 1160864
Comment on attachment 8600087 [details] [diff] [review]
patch v1

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

Wow, so many typos. We seriously need more tests here.
Attachment #8600087 - Flags: review?(ttaubert) → review+
Attachment #8600087 - Flags: review?(mano)
https://hg.mozilla.org/mozilla-central/rev/d0c13146f906
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
I'll leave this untracked for now since it's already fixed. Please renominate if it needs it! Thanks.
Verified fixed on Windows 7 64bit, Mac OSX 10.9.5 and Ubuntu 13.10 32bit using latest Aurora 40.0a2 (buildID: 20150526004004).
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.