Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#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 5 years ago.

Download all attachments as: .zip

Change History (11)

Changed 5 years ago by passbrains

comment:1 Changed 5 years ago 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 5 years ago by mapx

  • Cc mapx added
  • Description modified (diff)

comment:3 Changed 5 years ago 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 5 years ago 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 5 years ago 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 5 years ago by trev

  • Priority changed from Unknown to P3
  • Ready set

comment:7 Changed 5 years ago 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 5 years ago by sebastian

Also see #1343.

comment:9 Changed 5 years ago by greiner

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

comment:10 Changed 5 years ago by greiner

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next
Note: See TracTickets for help on using tickets.