1458890 - Change the host wildcard in RemoveByFilter

Status: RESOLVED FIXED

(Toolkit :: Places, enhancement, P2)

Description

[Marco Bonardo [:mak]]

See
https://bugzilla.mozilla.org/show_bug.cgi?id=1089691#c42
and
https://bugzilla.mozilla.org/show_bug.cgi?id=1089691#c44

The current wildcard creates a problem with hosts not having dots, it doesn't make much sense to pass *.localhost to clear localhost

Comment 1

[Marco Bonardo [:mak]]

Attachment: Bug 1458890 - Remove the '*' requirement from the host wildcard in History.removeByFilter.

Review commit: https://reviewboard.mozilla.org/r/242148/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/242148/

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 8973779 [details]
Bug 1458890 - Remove the '*' requirement from the host wildcard in History.removeByFilter.

https://reviewboard.mozilla.org/r/242148/#review248472

Looks good, now I understand it.

::: commit-message-8994f:1
(Diff revision 1)
> +Bug 1458890 - Change the host wildcard in History.removeByFilter. r=standard8

nit: maybe update the message to explain the change of wildcard (e.g. * to .)

::: toolkit/components/places/History.jsm:507
(Diff revision 1)
>      if (!hasBeginDate && !hasEndDate && !hasHost) {
>        throw new TypeError("Expected a non-empty filter");
>      }
>  
> -    // Host should follow one of these formats
> -    // The first one matches `localhost` or any other custom set in hostsfile
> +    // Check the host format.
> +    // Either it has not dots, or has multiple dots, or it's a single dot char.

nit: s/not/no/

::: toolkit/components/places/History.jsm:1167
(Diff revision 1)
>        hostFilterSQLFragment =
> -        `h.rev_host between :revHostStart and :revHostEnd`;
> +        `h.rev_host between :revHost || "." and :revHost || "/"`;
> -      params.revHostStart = revHost + ".";
> -      params.revHostEnd = revHost + "/";
>      } else {
>        // This covers the rest (mozilla.org, localhost and local files)

nit: comment needs updating as local files are covered by the first if.

Comment 3

[Marco Bonardo [:mak]]

Comment on attachment 8973779 [details]
Bug 1458890 - Remove the '*' requirement from the host wildcard in History.removeByFilter.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242148/diff/1-2/

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d5c99b70e34
Remove the '*' requirement from the host wildcard in History.removeByFilter. r=standard8

Comment 5

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/2d5c99b70e34