don't set browser.history.last_page_visited if not needed

RESOLVED FIXED in mozilla1.2alpha

Status

()

Core
History: Global
P3
normal
RESOLVED FIXED
17 years ago
16 years ago

People

(Reporter: Jeremy M. Dolan, Assigned: Christopher Aillon (sabbatical, not receiving bugmail))

Tracking

({perf})

Trunk
mozilla1.2alpha
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

17 years ago
Unless Nav is set to display the last page visited on startup,
browser.history.last_page_visited shouldn't be set or writen to the prefs, for
performance and privacy reasons. (user could clear cookies and cache, but the
pref will still be around)

Comment 1

17 years ago
This belongs to history. I'm not sure whether it's session or global though.
Guessing global...
Assignee: bnesse → blakeross
Component: Preferences: Backend → History: Global
QA Contact: sairuh → claudius

Updated

17 years ago
Target Milestone: --- → mozilla1.1
Taking, I have a patch in the works for this.  The perf issue really isn't that
big here since we have to either read or set the pref in this case.  However, we
can get a perf win by not getting a new pref service for each page load.  My
patch will do that too.
Assignee: blaker → caillon
Keywords: perf
Created attachment 93057 [details] [diff] [review]
Patch v.1.0

Uses our new pref APIs and caches the prefbranch we use so as not to keep
creating them for every page load.  Fixes this bug by checking the pref in C++
and not setting anything if the user doesn't want to track their last visited
page, and fixes JS code to do the right thing when we switch between prefs. 
Plus makes the buttons work as they should (Set to current page is disabled if
the home page is the current page, same for set to default).

Comment 4

16 years ago
Comment on attachment 93057 [details] [diff] [review]
Patch v.1.0

sr=blake
Attachment #93057 - Flags: superreview+

Comment 5

16 years ago
Comment on attachment 93057 [details] [diff] [review]
Patch v.1.0

sr=alecf
nice cleanup!

Comment 6

16 years ago
Comment on attachment 93057 [details] [diff] [review]
Patch v.1.0

oops, that sr was blake's
r=alecf
Attachment #93057 - Flags: review+
Drivers say this won't make 1.1.
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: mozilla1.1alpha → mozilla1.2alpha
Checked into trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.