bulk delete history by hostname, by domain

VERIFIED FIXED in mozilla0.8

Status

()

Core
History: Global
P2
normal
VERIFIED FIXED
18 years ago
16 years ago

People

(Reporter: Alec Flett, Assigned: Alec Flett)

Tracking

Trunk
mozilla0.8
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fix in hand)

Attachments

(2 attachments)

(Assignee)

Description

18 years ago
Since deleting individual items from history was such a well-recieved feature, I
decided to also implement the ability to delete history entries by hostname, and
by domain.. so that porn surfers could quickly and easily hide the traces of
their elicit wanderings from their loved ones.

patch forthcoming, looking for reviewers.
(Assignee)

Comment 1

18 years ago
Created attachment 21547 [details] [diff] [review]
proposed fix
(Assignee)

Comment 2

18 years ago
other random junk in the patch:
- cleaning up case sensitivity stuff
- fixing JS warnings

the basic parts of this are:
- C++ implementation of a matching function with matches on the hostname, or the
domain (can really be used to match any postfix that's passed in)
- JS controller to update everything.. I implemented the full controller because
I think we'll be adding more to history in the future
- JS glue to update the controller, change selection, and so forth.
- new strings to show stuff in the menu

I am open to suggestions on the strings - I just put some dummy ones in there.
Status: NEW → ASSIGNED

Comment 3

18 years ago
this is very cool.  one step closer to becoming the browser of choice for porn 
surfers everywhere...
OS: Linux → All
Hardware: PC → All
+    dump("Selection changed, have .." + selection.length + " items\n");
+    if (selection && selection.length > 0) {

That dump will throw an exception if selection is null -- split the if in two
and have the outer one guard the dump as well as the inner?

+        var match = gLastHostname.match(/([^\.]+\.[^\.]+$)/);

No need for \ before . in [] bracketed character classes.

+        switch(command)
+        {
+            case "cmd_deleteByHostname":
+            case "cmd_deleteByDomain":
+                return true;

Non-conforming brace-style alert!

Also, here and moreso in subsequent switches, burning an entire four-space
indentation level for the case labels tends to overindent code and spread it out
horizontally too much.  Why not use half-stop indentation (two spaces)?  This is
a total style nit, I know, and it doesn't come up with two-space indentation
stops, but it bugged me so I thought I'd mention it.

+            case "cmd_deleteByDomain":
+            dump("Removing all domain from " + gLastDomain + "\n");

Indentation glitch.

I didn't finish reviewing, but I wanted to update now (I feel a crash coming
on ;-), and hope you can use these comments.

/be
(Assignee)

Comment 5

18 years ago
oops, that dump wasn't supposed to be in there at all...

I'll fix the other stuff too, like the regexp thing.. as for the brace issues, I
just do what emacs indents for me in Java mode (oh, to have a JS mode for
emacs...*sigh*) but I'll fix it up.

I'll fix those up, and attach a new patch..
(Assignee)

Updated

18 years ago
Priority: -- → P2
Target Milestone: --- → mozilla0.8
(Assignee)

Comment 6

18 years ago
oops, still haven't attached the new patch, but putting fix in hand, so I can
keep my buglist straight
Whiteboard: fix in hand
(Assignee)

Comment 7

18 years ago
Created attachment 23360 [details] [diff] [review]
updated patch
(Assignee)

Comment 8

18 years ago
I now have a patch, looking for reviewer/super reviewer.
Keywords: patch

Comment 9

18 years ago
what's the story on the BookmarksTree name?

odd indentation: [tab?]
+        gHistoryTree.selectItem(children.firstChild);
+    gHistoryTree.focus();

Curiousity, why is this signed:
PRInt64* expirationDate

your function definition newline style is inconsistent:
+nsGlobalHistory::MatchHost(nsIMdbRow *aRow,
+                           matchHost_t *hostInfo)

your code looks good, but i'm sure you don't want an r=timeless.
Keywords: approval, review
(Assignee)

Comment 10

18 years ago
- still trying to exorcise the bookmarkstree name
- Indentation fixed in my tree, whoops
- signed because a NSPR uses PRInt64 - using unsigned would requiring annoying
and unnecessary casting.
- newline indentation looks fine to me. emacs agrees with me.
Time numbers must be negative for dates before 1970, and to hold time number
differences.  Plus, casting between unsigned and signed is rarely useful (so
much so that Java dispensed with unsigned types, but that hurds division and
modulus counterexamples, and valid unsigned range tests).

/be
(Assignee)

Updated

18 years ago
Component: History: Session → History: Global
(Assignee)

Updated

18 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
(Assignee)

Comment 13

18 years ago
fix is in

Comment 14

18 years ago
Posted bug 67387 for adding mnemonics for the new commands.
mass-verifying claudius' Fixed bugs which haven't changed since 2001.12.31.

if you think this particular bug is not fixed, please make sure of the following
before reopening:

a. retest with a *recent* trunk build.
b. query bugzilla to see if there's an existing, open bug (new, reopened,
assigned) that covers your issue.
c. if this does need to be reopened, make sure there are specific steps to
reproduce (unless already provided and up-to-date).

thanks!

[set your search string in mail to "AmbassadorKoshNaranek" to filter out these
messages.]
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.