Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(116)

Issue 5173617369284608: Issue 1868 - Handle <area> element properly when selected with "Block element" (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years ago by Sebastian Noack
Modified:
5 years ago
Reviewers:
kzar
CC:
Wladimir Palant
Visibility:
Public.

Description

Injecting overlays for images only work when using "Block element" from the icon popup, however not when using the context menu. No overlays are injected yet, when the context menu is used. Therefore we have to traverse the tree to find the image associated with the <area> element.

Patch Set 1 : #

Total comments: 5

Patch Set 2 : Rebased and fixed grammar in comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+40 lines, -10 lines) Patch
M include.postload.js View 1 3 chunks +40 lines, -10 lines 0 comments Download

Messages

Total messages: 4
Sebastian Noack
5 years ago (2015-01-28 10:12:17 UTC) #1
kzar
http://codereview.adblockplus.org/5173617369284608/diff/5757334940811264/include.postload.js File include.postload.js (right): http://codereview.adblockplus.org/5173617369284608/diff/5757334940811264/include.postload.js#newcode337 include.postload.js:337: // Add overlays for blockable elements that don't emit ...
5 years ago (2015-02-09 17:24:40 UTC) #2
Sebastian Noack
http://codereview.adblockplus.org/5173617369284608/diff/5757334940811264/include.postload.js File include.postload.js (right): http://codereview.adblockplus.org/5173617369284608/diff/5757334940811264/include.postload.js#newcode337 include.postload.js:337: // Add overlays for blockable elements that don't emit ...
5 years ago (2015-02-10 10:19:27 UTC) #3
kzar
5 years ago (2015-02-10 11:03:00 UTC) #4
LGTM

http://codereview.adblockplus.org/5173617369284608/diff/5757334940811264/incl...
File include.postload.js (right):

http://codereview.adblockplus.org/5173617369284608/diff/5757334940811264/incl...
include.postload.js:424: var usemap = image.getAttribute("usemap");
On 2015/02/10 10:19:27, Sebastian Noack wrote:
> On 2015/02/09 17:24:41, kzar wrote:
> > Don't we need to check that usemap isn't null before we try to use its
indexOf
> > and substr methods?
> 
> Mind the CSS selector above, excluding elements without "usemap" attribute.

Ah OK
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5