Opened on 09/18/2014 at 10:06:53 AM

Closed on 09/25/2014 at 09:52:13 AM

Last modified on 09/25/2014 at 10:09:55 AM

#1393 closed defect (fixed)

JPG image embedded does not get blocked if there is a space in image name

Reported by: passbrains Assignee: greiner
Priority: P3 Milestone: Adblock-Plus-1.8.6-for-Chrome-Opera-Safari
Module: User-Interface Keywords:
Cc: mapx, trev Blocked By:
Blocking: Platform: Opera
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

http://codereview.adblockplus.org/4567408790470656/

Description (last modified by mapx)

Adapted from https://www.passbrains.com/dashboard/view-ticket.php?ticket_no=AOP-84

Environment

Windows Vista 64bit Opera English
ABP version Adblock Plus Version 1.8.5.1227

How to reproduce

  1. Install ABP extension on Opera.
  1. Loads website  http://www.deccanherald.com
  2. Right click on the Sun image (as shown in attached screenshot by red circle)  and select 'Block Element' option to block the image.
  3. Add the filter displayed.

Filter used is : http://www.deccanherald.com/images/Sunny day.jpg
Image would be blocked.

  1. Reload the page as in step 2.

 

Observed behaviour

Jpg image "Sunny day.jpg" does not get blocked.
(Analysis  : This may be due to space in name of image)

Expected behaviour

jpg image shall be blocked.
 

Attachments (1)

3659_1410975216_sunny-image1.jpg (181.1 KB) - added by passbrains on 09/18/2014 at 10:06:56 AM.

Download all attachments as: .zip

Change History (11)

Changed on 09/18/2014 at 10:06:56 AM by passbrains

comment:1 Changed on 09/18/2014 at 10:06:59 AM by passbrains

1 - 18 Sep 2014 10:06:42 posted by Philip Hill
The filter suggested should include the whitespace in the form of instead.

comment:2 Changed on 09/18/2014 at 11:38:17 AM by mapx

  • Cc mapx added
  • Description modified (diff)

comment:3 Changed on 09/18/2014 at 02:22:57 PM by greiner

  • Component changed from Unknown to Platform
  • Owner set to greiner

Actually, the image URL is "http://www.deccanherald.com/images/Sunny%20day.jpg" but the block element dialog suggests a filter with a whitespace in it instead. If the correct filter is added it does work but even then the filter is shown with a whitespace in the options page so we should first evaluate which areas of the extension are affected by this.

comment:4 Changed on 09/18/2014 at 03:02:06 PM by philll

@greiner: Yes, that's what I initally commented in the passbrains issue. Unfortunately, they didn't escape it correctly, such that my "%20" didn't even make it into their comment section. Informed them about this bug.

comment:5 Changed on 09/18/2014 at 03:13:05 PM by greiner

  • Component changed from Platform to User-Interface
  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

It turned out to be limited to the block element feature after all.

comment:6 Changed on 09/18/2014 at 06:53:50 PM by trev

  • Priority changed from Unknown to P3
  • Ready set

comment:7 Changed on 09/18/2014 at 07:03:11 PM by trev

  • Cc trev added

It's actually far from trivial. See http://hg.mozilla.org/mozilla-central/file/426497473505/xpcom/io/nsEscape.cpp#l314 for Mozilla's NS_EscapeURL() function, Chrome must have something similar. This determines which characters are being escaped automatically and where. Note that Chrome's implementation is most definitely different - e.g. Firefox escapes ' while Chrome doesn't.

comment:8 Changed on 09/24/2014 at 11:38:23 AM by sebastian

Also see #1343.

comment:9 Changed on 09/25/2014 at 09:52:13 AM by greiner

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:10 Changed on 09/25/2014 at 10:09:55 AM by greiner

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from greiner.
 
Note: See TracTickets for help on using tickets.