Bug 159485

Summary:

[Content Filtering] Load blocked pages more like other error pages are loaded

Product:

WebKit

Reporter:

Andy Estes <aestes>

Component:

New Bugs

Assignee:

Andy Estes <aestes>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, beidson, buildbot, cdumez, commit-queue, darin, japhet, kling, mitz, rniwa, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

159480, 159570

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Archive of layout-test-results from ews101 for mac-yosemite	none
Archive of layout-test-results from ews125 for ios-simulator-wk2	none
Archive of layout-test-results from ews104 for mac-yosemite-wk2	none
Archive of layout-test-results from ews116 for mac-yosemite	none
Patch	none
Patch	none
Patch	none
Patch	none
Differences between attachment 283189 and r202944	none

Andy Estes

Reported 2016-07-06 12:29:39 PDT

[Content Filtering] Load blocked pages more like other error pages are loaded

Attachments
Patch (118.07 KB, patch) 2016-07-06 13:20 PDT, Andy Estes	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews101 for mac-yosemite (828.53 KB, application/zip) 2016-07-06 14:14 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews125 for ios-simulator-wk2 (916.48 KB, application/zip) 2016-07-06 14:15 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (939.59 KB, application/zip) 2016-07-06 14:17 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews116 for mac-yosemite (1.45 MB, application/zip) 2016-07-06 14:30 PDT, Build Bot	no flags	Details
Patch (118.19 KB, patch) 2016-07-06 17:48 PDT, Andy Estes	no flags	Details Formatted Diff Diff
Patch (118.19 KB, patch) 2016-07-06 18:35 PDT, Andy Estes	no flags	Details Formatted Diff Diff
Patch (118.28 KB, patch) 2016-07-07 15:56 PDT, Andy Estes	no flags	Details Formatted Diff Diff
Patch (120.28 KB, patch) 2016-07-08 12:37 PDT, Andy Estes	no flags	Details Formatted Diff Diff
Differences between attachment 283189 and r202944 (2.19 KB, patch) 2016-07-08 12:39 PDT, Andy Estes	no flags	Details Formatted Diff Diff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.

Andy Estes

Comment 1 2016-07-06 12:31:33 PDT

<rdar://problem/26014076>

Andy Estes

Comment 2 2016-07-06 13:20:09 PDT

Created attachment 282937 [details] Patch

WebKit Commit Bot

Comment 3 2016-07-06 13:21:52 PDT

Attachment 282937 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WebKit2Cocoa/ContentFiltering.mm:274: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 55 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 4 2016-07-06 14:14:25 PDT

Comment on attachment 282937 [details] Patch Attachment 282937 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1637379 New failing tests: contentfiltering/allow-after-will-send-request.html contentfiltering/allow-never.html contentfiltering/allow-after-add-data.html contentfiltering/allow-after-finished-adding-data.html contentfiltering/block-never.html contentfiltering/allow-after-response.html

Build Bot

Comment 5 2016-07-06 14:14:30 PDT

Created attachment 282945 [details] Archive of layout-test-results from ews101 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5

Build Bot

Comment 6 2016-07-06 14:15:23 PDT

Comment on attachment 282937 [details] Patch Attachment 282937 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1637354 New failing tests: contentfiltering/allow-after-will-send-request.html contentfiltering/allow-never.html contentfiltering/block-after-response-then-deny-unblock.html contentfiltering/allow-after-add-data.html contentfiltering/block-after-finished-adding-data-then-deny-unblock.html contentfiltering/block-after-will-send-request-then-allow-unblock.html contentfiltering/block-after-add-data.html contentfiltering/allow-after-finished-adding-data.html contentfiltering/block-after-add-data-then-deny-unblock.html contentfiltering/block-after-will-send-request-then-deny-unblock.html contentfiltering/block-after-add-data-then-allow-unblock.html contentfiltering/block-after-response.html contentfiltering/block-after-response-then-allow-unblock.html contentfiltering/block-after-will-send-request.html contentfiltering/block-after-finished-adding-data.html contentfiltering/block-never.html contentfiltering/block-after-finished-adding-data-then-allow-unblock.html contentfiltering/allow-after-response.html

Build Bot

Comment 7 2016-07-06 14:15:27 PDT

Created attachment 282946 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.4

Build Bot

Comment 8 2016-07-06 14:17:02 PDT

Comment on attachment 282937 [details] Patch Attachment 282937 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1637378 New failing tests: contentfiltering/block-never.html contentfiltering/allow-after-will-send-request.html contentfiltering/allow-never.html contentfiltering/block-after-response-then-deny-unblock.html contentfiltering/allow-after-add-data.html contentfiltering/block-after-finished-adding-data-then-deny-unblock.html contentfiltering/block-after-will-send-request-then-allow-unblock.html contentfiltering/block-after-add-data.html contentfiltering/allow-after-finished-adding-data.html contentfiltering/block-after-add-data-then-deny-unblock.html contentfiltering/block-after-add-data-then-allow-unblock.html contentfiltering/block-after-response.html contentfiltering/block-after-response-then-allow-unblock.html contentfiltering/block-after-will-send-request.html contentfiltering/block-after-finished-adding-data.html contentfiltering/block-after-will-send-request-then-deny-unblock.html contentfiltering/block-after-finished-adding-data-then-allow-unblock.html contentfiltering/allow-after-response.html

Build Bot

Comment 9 2016-07-06 14:17:07 PDT

Created attachment 282947 [details] Archive of layout-test-results from ews104 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5

Build Bot

Comment 10 2016-07-06 14:30:50 PDT

Comment on attachment 282937 [details] Patch Attachment 282937 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1637380 New failing tests: contentfiltering/allow-after-will-send-request.html contentfiltering/allow-never.html contentfiltering/allow-after-add-data.html contentfiltering/allow-after-finished-adding-data.html contentfiltering/block-never.html contentfiltering/allow-after-response.html

Build Bot

Comment 11 2016-07-06 14:30:55 PDT

Created attachment 282948 [details] Archive of layout-test-results from ews116 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5

Andy Estes

Comment 12 2016-07-06 14:32:36 PDT

The test failures are due to https://bugs.webkit.org/show_bug.cgi?id=159480, which is blocking this bug.

Andy Estes

Comment 13 2016-07-06 17:48:38 PDT

Created attachment 282971 [details] Patch

WebKit Commit Bot

Comment 14 2016-07-06 17:50:47 PDT

Attachment 282971 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WebKit2Cocoa/ContentFiltering.mm:274: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 55 files If any of these errors are false positives, please file a bug against check-webkit-style.

Andy Estes

Comment 15 2016-07-06 18:35:22 PDT

Created attachment 282979 [details] Patch

WebKit Commit Bot

Comment 16 2016-07-06 18:38:13 PDT

Attachment 282979 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WebKit2Cocoa/ContentFiltering.mm:274: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 55 files If any of these errors are false positives, please file a bug against check-webkit-style.

Alex Christensen

Comment 17 2016-07-06 22:41:25 PDT

Comment on attachment 282979 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=282979&action=review Glancing through it. Seems reasonable. Needs more thorough review, though. > Source/WebCore/loader/ContentFilter.cpp:263 > +bool ContentFilter::continueAfterSubstituteDataRequest(DocumentLoader* activeLoader, const SubstituteData& substituteData) activeLoader could be a DocumentLoader&

Brady Eidson

Comment 18 2016-07-07 15:13:15 PDT

Comment on attachment 282979 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=282979&action=review The code here looks fine, but I spotted what I think is a showstopper. > Source/WebCore/Resources/ContentFilterBlockedPage.html:3 > +<!DOCTYPE html> > +<meta charset=utf-8> > +<title>Blocked by Content Filter</title> This concerns me. It's the first framework resource that has user visible text in it (The title is user visible for most users) What is our localization strategy.

Brady Eidson

Comment 19 2016-07-07 15:21:31 PDT

We discussed in person - The file is a placeholder. It should just have an html comment that says "this file intentionally left blank"

Brady Eidson

Comment 20 2016-07-07 15:21:48 PDT

Comment on attachment 282979 [details] Patch R+ with that change

Andy Estes

Comment 21 2016-07-07 15:56:36 PDT

Created attachment 283074 [details] Patch

Andy Estes

Comment 22 2016-07-07 16:00:53 PDT

(In reply to comment #17) > Comment on attachment 282979 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=282979&action=review > > > Source/WebCore/loader/ContentFilter.cpp:263 > > +bool ContentFilter::continueAfterSubstituteDataRequest(DocumentLoader* activeLoader, const SubstituteData& substituteData) > > activeLoader could be a DocumentLoader& Good call! I made it a const DocumentLoader&.

WebKit Commit Bot

Comment 23 2016-07-07 16:27:08 PDT

Comment on attachment 283074 [details] Patch Clearing flags on attachment: 283074 Committed r202944: <http://trac.webkit.org/changeset/202944>

WebKit Commit Bot

Comment 24 2016-07-07 16:27:15 PDT

All reviewed patches have been landed. Closing bug.

WebKit Commit Bot

Comment 25 2016-07-08 11:20:15 PDT

Re-opened since this is blocked by bug 159570

Andy Estes

Comment 26 2016-07-08 12:37:21 PDT

Created attachment 283189 [details] Patch

WebKit Commit Bot

Comment 27 2016-07-08 12:38:54 PDT

Attachment 283189 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/Tests/WebKit2Cocoa/ContentFiltering.mm:274: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 55 files If any of these errors are false positives, please file a bug against check-webkit-style.

Andy Estes

Comment 28 2016-07-08 12:39:02 PDT

Created attachment 283190 [details] Differences between attachment 283189 [details] and r202944

WebKit Commit Bot

Comment 29 2016-07-08 13:15:15 PDT

Comment on attachment 283189 [details] Patch Clearing flags on attachment: 283189 Committed r203003: <http://trac.webkit.org/changeset/203003>

WebKit Commit Bot

Comment 30 2016-07-08 13:15:21 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.