Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29755575: Issue 6595 - Make sure frame structure gets always updated (Closed)

Created:
April 18, 2018, 11:16 a.m. by Sebastian Noack
Modified:
May 2, 2018, 4:29 p.m.
Reviewers:
kzar
CC:
Manish Jethani
Visibility:
Public.

Description

Issue 6595 - Make sure frame structure gets always updated

Patch Set 1 #

Total comments: 9
Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -12 lines) Patch
M ext/background.js View 1 chunk +9 lines, -12 lines 9 comments Download

Messages

Total messages: 7
Sebastian Noack
https://codereview.adblockplus.org/29755575/diff/29755576/ext/background.js File ext/background.js (left): https://codereview.adblockplus.org/29755575/diff/29755576/ext/background.js#oldcode269 ext/background.js:269: url.startsWith("https:") && The assumption that every document with an ...
April 18, 2018, 11:25 a.m. (2018-04-18 11:25:36 UTC) #1
kzar
https://codereview.adblockplus.org/29755575/diff/29755576/ext/background.js File ext/background.js (left): https://codereview.adblockplus.org/29755575/diff/29755576/ext/background.js#oldcode269 ext/background.js:269: url.startsWith("https:") && On 2018/04/18 11:25:36, Sebastian Noack wrote: > ...
May 2, 2018, 12:06 p.m. (2018-05-02 12:06:52 UTC) #2
Sebastian Noack
https://codereview.adblockplus.org/29755575/diff/29755576/ext/background.js File ext/background.js (left): https://codereview.adblockplus.org/29755575/diff/29755576/ext/background.js#oldcode269 ext/background.js:269: url.startsWith("https:") && On 2018/05/02 12:06:52, kzar wrote: > But ...
May 2, 2018, 2:32 p.m. (2018-05-02 14:32:16 UTC) #3
kzar
Assuming you tested that things don't explode in Chrome when the onCommitted listener fires for ...
May 2, 2018, 3:26 p.m. (2018-05-02 15:26:53 UTC) #4
Sebastian Noack
https://codereview.adblockplus.org/29755575/diff/29755576/ext/background.js File ext/background.js (left): https://codereview.adblockplus.org/29755575/diff/29755576/ext/background.js#oldcode269 ext/background.js:269: url.startsWith("https:") && On 2018/05/02 15:26:52, kzar wrote: > I ...
May 2, 2018, 3:34 p.m. (2018-05-02 15:34:49 UTC) #5
kzar
https://codereview.adblockplus.org/29755575/diff/29755576/ext/background.js File ext/background.js (left): https://codereview.adblockplus.org/29755575/diff/29755576/ext/background.js#oldcode269 ext/background.js:269: url.startsWith("https:") && On 2018/05/02 15:34:49, Sebastian Noack wrote: > ...
May 2, 2018, 3:46 p.m. (2018-05-02 15:46:35 UTC) #6
Sebastian Noack
May 2, 2018, 4:27 p.m. (2018-05-02 16:27:55 UTC) #7
https://codereview.adblockplus.org/29755575/diff/29755576/ext/background.js
File ext/background.js (left):

https://codereview.adblockplus.org/29755575/diff/29755576/ext/background.js#o...
ext/background.js:269: url.startsWith("https:") &&
On 2018/05/02 15:46:35, kzar wrote:
> On 2018/05/02 15:34:49, Sebastian Noack wrote:
> > Anyway, do you have a website at hand that shows frames with
> > content loaded from a blob: URL? Then I can quickly test what happens.
> 
> Well if you paste this snippet from my Chromium issue into the console for a
> page (which doesn't have a CSP that clashes) you'll be able to see what
happens:
> 
> let frame = document.createElement("iframe");
> frame.src = URL.createObjectURL(new Blob([""]));
> document.body.appendChild(frame)

I tested it, and the onCommitted listener is emitted for the frame with blob:
URL causing the frame to be recorded (as expected), which has no visible effect.

I will push the changes now to a "next" bookmark.

Powered by Google App Engine
This is Rietveld