moz_openpages_temp table needs to describe pages that may not be in moz_places

RESOLVED FIXED in mozilla2.0b4

Status

()

Toolkit
Places
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: Unfocused, Assigned: mak)

Tracking

(Blocks: 1 bug)

unspecified
mozilla2.0b4
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Followup for bug 480350.

The moz_openpages_temp currently joins with moz_places based on ID. However, if an open page has been removed from history, then it won't register as being open, since there will be no corresponding row in moz_places to join with. 

To solve this, moz_openpages_temp could instead hold URL and title, and join based on URL.
Blocks: 546254
Duplicate of this bug: 558368
Blocks: 558321
Blocks: 558841
blocking2.0: --- → ?
(Assignee)

Updated

7 years ago
Duplicate of this bug: 562941
blocking2.0: ? → beta1+
Blocks: 480350
No longer depends on: 480350
Blocks: 558626
Blocks: 558330
Definitely blocks, but we wouldn't hold beta1 for it
blocking2.0: beta1+ → final+

Comment 4

7 years ago
is this also relevant for switching to "about:" pages, or should i file a separate bug for that?
(Assignee)

Comment 5

7 years ago
this fix should work for about: pages afaict.
(Assignee)

Updated

7 years ago
Blocks: 585966
(Assignee)

Comment 6

7 years ago
stealing for now, I need this sooner than later, due to add page contentions with async visitURI
Assignee: bmcbride → mak77
(In reply to comment #6)
> stealing for now, I need this sooner than later, due to add page contentions
> with async visitURI
Can you updated the dependencies?
(Assignee)

Comment 8

7 years ago
(In reply to comment #7)
> (In reply to comment #6)
> > stealing for now, I need this sooner than later, due to add page contentions
> > with async visitURI
> Can you updated the dependencies?

bug 585966 is already in the blocks section of this bug, could also help bug 559217
Blocks: 559217
(Assignee)

Comment 9

7 years ago
sorry I meant bug 563538
Blocks: 563538
No longer blocks: 559217
(Assignee)

Comment 10

7 years ago
Created attachment 464653 [details] [diff] [review]
patch v1.0
(Assignee)

Comment 11

7 years ago
Comment on attachment 464653 [details] [diff] [review]
patch v1.0

this also moves up results in the list, that was requested in another bug (but was easier to do it here).
The backend (register/unregister) part is something we want regardless, the ordering of the results changes here and would be nice to get users feedback.
Attachment #464653 - Flags: review?(sdwilsh)
Comment on attachment 464653 [details] [diff] [review]
patch v1.0

r=sdwilsh
Attachment #464653 - Flags: review?(sdwilsh) → review+
(Assignee)

Comment 13

7 years ago
there is probably some glitch with about:blank registration since I see a related failure on tryserver:
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1281549452.1281550402.29118.gz TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/base/content/test/browser_tabMatchesInAwesomebar.js | tab is open (1 times) and should recorded in db: about:blank
Or it's just a timing issue of the test that should wait visits addition (bug 576605 has a wip patch for that).
(Assignee)

Updated

7 years ago
Depends on: 586506
(Assignee)

Comment 14

7 years ago
the test failure is caused by bug 586506 (browser_bug380960.js is the origin of the unmatching tab).
(Assignee)

Comment 15

7 years ago
Created attachment 465797 [details] [diff] [review]
patch v1.1

switch to tab was covering a missing change in docshell, will try to get a diff review
Attachment #464653 - Attachment is obsolete: true
(Assignee)

Comment 16

7 years ago
Created attachment 465801 [details] [diff] [review]
patch v1.2

just a fix to a comment.
Attachment #465797 - Attachment is obsolete: true
(Assignee)

Comment 17

7 years ago
Comment on attachment 465801 [details] [diff] [review]
patch v1.2

asking review on the small dochell change, the design of switch to tab was covering the fact docshell was still trying to set title synchronously before adding a page.
Attachment #465801 - Flags: review?(sdwilsh)
Comment on attachment 465801 [details] [diff] [review]
patch v1.2

r=sdwilsh on the docshell changes
Attachment #465801 - Flags: review?(sdwilsh) → review+
http://hg.mozilla.org/mozilla-central/rev/469328928474
(Assignee)

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b4
(Assignee)

Updated

7 years ago
Blocks: 546255

Updated

7 years ago
Blocks: 607894
You need to log in before you can comment on or make changes to this bug.