Review Request: khtml adblock filter lookup API

[Jonathan Marten-3]

Dear all,

I've seen it suggested on another ML that here is the place to discuss
KHTML changes/additions.  If you weren't aware of it already, would it
be possible for an expert to look at http://reviewboard.kde.org/r/1382
and give a yes/no whether it is acceptable?

This is an addition to the KHTML API (binary and source compatible) to
allow the adblock filter matching a URL to be identified - the current
API provides only a boolean match.  This is so that the matching
filter can be shown in the adblock dialogue, allowing the filter that
caused something to be blocked to be identified.  There should be no
performance impact under normal operation.

Many thanks,
  Jonathan



--
Jonathan Marten                         http://www.keelhaul.demon.co.uk
Twickenham, UK                          jjm2@...

[Maksim Orlovich-2]

> Dear all,
>
> I've seen it suggested on another ML that here is the place to discuss
> KHTML changes/additions.  If you weren't aware of it already, would it
> be possible for an expert to look at http://reviewboard.kde.org/r/1382
> and give a yes/no whether it is acceptable?
>
> This is an addition to the KHTML API (binary and source compatible) to
> allow the adblock filter matching a URL to be identified - the current
> API provides only a boolean match.  This is so that the matching
> filter can be shown in the adblock dialogue, allowing the filter that
> caused something to be blocked to be identified.  There should be no
> performance impact under normal operation.

Looks fine, though I can't be 100% sure that what it returns in complex
cases is identical to the input.

Sorry for missing your previous e-mail.

[Jonathan Marten-3]

On Monday 21 September 2009 15:53:09, "Maksim Orlovich" <mo85@...>
wrote:
> Looks fine, though I can't be 100% sure that what it returns in complex
> cases is identical to the input.
>
> Sorry for missing your previous e-mail.

Many thanks Maksim, I'll commit with these changes and verify them with the
modified plugin.  As far as I can tell the reported filter is the same as the
input except in the case where the original filter was in the form
http://host.dom/path/*, as an optimisation the trailing "/*" is stripped off
so this becomes just a simple
string match.  There's no way to tell that this
has happened for display, though.  The important part - the host and path -
is correctly reported.

Regards,
  Jonathan

--
Jonathan Marten                         http://www.keelhaul.demon.co.uk
Twickenham, UK                          jjm2@...