Bug 159485 - [Content Filtering] Load blocked pages more like other error pages are loaded
Summary: [Content Filtering] Load blocked pages more like other error pages are loaded
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andy Estes
URL:
Keywords: InRadar
Depends on: 159480 159570
Blocks:
  Show dependency treegraph
 
Reported: 2016-07-06 12:29 PDT by Andy Estes
Modified: 2016-07-08 13:15 PDT (History)
11 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Estes 2016-07-06 12:29:39 PDT
[Content Filtering] Load blocked pages more like other error pages are loaded
Comment 1 Andy Estes 2016-07-06 12:31:33 PDT
<rdar://problem/26014076>
Comment 2 Andy Estes 2016-07-06 13:20:09 PDT
Created attachment 282937 [details]
Patch
Comment 3 WebKit Commit Bot 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.
Comment 4 Build Bot 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
Comment 5 Build Bot 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
Comment 6 Build Bot 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
Comment 7 Build Bot 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
Comment 8 Build Bot 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
Comment 9 Build Bot 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
Comment 10 Build Bot 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
Comment 11 Build Bot 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
Comment 12 Andy Estes 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.
Comment 13 Andy Estes 2016-07-06 17:48:38 PDT
Created attachment 282971 [details]
Patch
Comment 14 WebKit Commit Bot 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.
Comment 15 Andy Estes 2016-07-06 18:35:22 PDT
Created attachment 282979 [details]
Patch
Comment 16 WebKit Commit Bot 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.
Comment 17 Alex Christensen 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&
Comment 18 Brady Eidson 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.
Comment 19 Brady Eidson 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"
Comment 20 Brady Eidson 2016-07-07 15:21:48 PDT
Comment on attachment 282979 [details]
Patch

R+ with that change
Comment 21 Andy Estes 2016-07-07 15:56:36 PDT
Created attachment 283074 [details]
Patch
Comment 22 Andy Estes 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&.
Comment 23 WebKit Commit Bot 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>
Comment 24 WebKit Commit Bot 2016-07-07 16:27:15 PDT
All reviewed patches have been landed.  Closing bug.
Comment 25 WebKit Commit Bot 2016-07-08 11:20:15 PDT
Re-opened since this is blocked by bug 159570
Comment 26 Andy Estes 2016-07-08 12:37:21 PDT
Created attachment 283189 [details]
Patch
Comment 27 WebKit Commit Bot 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.
Comment 28 Andy Estes 2016-07-08 12:39:02 PDT
Created attachment 283190 [details]
Differences between attachment 283189 [details] and r202944
Comment 29 WebKit Commit Bot 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>
Comment 30 WebKit Commit Bot 2016-07-08 13:15:21 PDT
All reviewed patches have been landed.  Closing bug.