Closed Bug 806742 Opened 7 years ago Closed 7 years ago

See if toolkit/components/places/tests/unit/test_248970.js makes sense in the new PB world

Categories

(Firefox :: Private Browsing, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 20

People

(Reporter: ehsan, Assigned: andreshm)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

No description provided.
I'm making changes to this test in bug 723005 to make it pass.
Depends on: 723005
So, Josh, should we rewrite this test to be a browser-chrome test which navigates to a bunch of pages in a private window, set a bookmark, etc.?
That would be good, yeah.
Assignee: josh → nobody
Assignee: nobody → andres
Attached patch Patch v1 (obsolete) — Splinter Review
I converted the test to a browser test. However, it's not really doing anything that needs windows, since all is based on PlaceUtils.
Attachment #684498 - Flags: review?(ehsan)
Comment on attachment 684498 [details] [diff] [review]
Patch v1

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

You need to access PlacesUtils from the private and non-private windows that you open.
Attachment #684498 - Flags: review?(ehsan) → review-
(In reply to Ehsan Akhgari [:ehsan] from comment #5)
> 
> You need to access PlacesUtils from the private and non-private windows that
> you open.

Do you mean aWindow.PlacesUtils? I think is the same, since PlacesUtils is a jsm and PlacesUtils.history and PlacesUtils.bookmarks are quick access to components, right?
(In reply to comment #6)
> (In reply to Ehsan Akhgari [:ehsan] from comment #5)
> > 
> > You need to access PlacesUtils from the private and non-private windows that
> > you open.
> 
> Do you mean aWindow.PlacesUtils? I think is the same, since PlacesUtils is a
> jsm and PlacesUtils.history and PlacesUtils.bookmarks are quick access to
> components, right?

Yes and yes.  I mean, this whole test is pretty pointless in the per-window world to be honest, but I don't think that we have anything else testing this stuff. :/
Attached patch Patch v2 (obsolete) — Splinter Review
Attachment #684498 - Attachment is obsolete: true
Attachment #684714 - Flags: review?(ehsan)
Attachment #684714 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/ac304d3c250e
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
This test fails on the birch branch: https://tbpl.mozilla.org/php/getParsedLog.php?id=17318126&tree=Birch
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch Patch v3Splinter Review
This patch should fix the total items count issue.
Attachment #684714 - Attachment is obsolete: true
Attachment #685284 - Flags: review?(ehsan)
Attachment #685284 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/db514198f30c
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.