WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
68208
Sites that use history pushState or replaceState are recorded in history in Private Browsing mode
https://bugs.webkit.org/show_bug.cgi?id=68208
Summary
Sites that use history pushState or replaceState are recorded in history in P...
Jessie Berlin
Reported
2011-09-15 19:10:18 PDT
Neither HistoryController::pushState nor HistoryController::replaceState check if private browsing is enabled before adding visited links and calling FrameLoaderClient::updateGlobalHistory. <
rdar://problem/9945573
>
Attachments
Patch
(1.98 KB, patch)
2011-09-15 19:21 PDT
,
Jessie Berlin
no flags
Details
Formatted Diff
Diff
Patch (take 2 - with API test)
(12.86 KB, patch)
2011-09-16 18:11 PDT
,
Jessie Berlin
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Jessie Berlin
Comment 1
2011-09-15 19:21:36 PDT
Created
attachment 107585
[details]
Patch
Andy Estes
Comment 2
2011-09-15 20:47:19 PDT
Can a test not be written for this?
Brady Eidson
Comment 3
2011-09-15 21:34:41 PDT
I'm not convinced this is actually a WebKit bug. Shouldn't this be a client app policy?
Jessie Berlin
Comment 4
2011-09-16 07:52:36 PDT
(In reply to
comment #2
)
> Can a test not be written for this?
I was trying to come up with ways to test this. I thought about a TestWebKitAPI test, but realized that I would be trying to assert that the WKContextDidNavigateWithNavigationDataCallback did not happen when Private Browsing is enabled - but there would be no way to determine when it was done “not happening”. Do you have any suggestions?
Jessie Berlin
Comment 5
2011-09-16 07:53:47 PDT
(In reply to
comment #3
)
> I'm not convinced this is actually a WebKit bug. Shouldn't this be a client app policy?
This checking for privateBrowsingEnabled before calling addVisitedLink and updateGlobalHistory is done throughout HistoryController. I don’t see why these should be the exception.
Brady Eidson
Comment 6
2011-09-16 08:16:11 PDT
(In reply to
comment #5
)
> (In reply to
comment #3
) > > I'm not convinced this is actually a WebKit bug. Shouldn't this be a client app policy? > > This checking for privateBrowsingEnabled before calling addVisitedLink and updateGlobalHistory is done throughout HistoryController. I don’t see why these should be the exception.
I guess it shouldn't be. But I think those were a bad call. They're leftover from when WebKit did history itself. oh well.
Andy Estes
Comment 7
2011-09-16 08:51:35 PDT
(In reply to
comment #4
)
> Do you have any suggestions?
You could write a layout test that asserts that links aren't considered visited after a pushState with private browsing enabled. I believe there is a special LTC method to get the visited state of a link.
Jessie Berlin
Comment 8
2011-09-16 09:16:16 PDT
Comment on
attachment 107585
[details]
Patch I am going to try out Andy's suggestion for a test.
Jessie Berlin
Comment 9
2011-09-16 18:11:36 PDT
Created
attachment 107759
[details]
Patch (take 2 - with API test)
WebKit Review Bot
Comment 10
2011-09-17 16:53:30 PDT
Comment on
attachment 107759
[details]
Patch (take 2 - with API test)
Attachment 107759
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/9725662
New failing tests: svg/W3C-SVG-1.1/animate-elem-12-t.svg svg/W3C-SVG-1.1/animate-elem-04-t.svg svg/W3C-SVG-1.1/animate-elem-07-t.svg platform/chromium/fast/text/text-stroke-with-border.html svg/W3C-SVG-1.1/animate-elem-08-t.svg svg/W3C-SVG-1.1/animate-elem-09-t.svg printing/return-from-printing-mode.html platform/mac-snowleopard/platform/mac/fast/text/international/Geeza-Pro-vertical-metrics-adjustment.html svg/W3C-SVG-1.1/animate-elem-11-t.svg svg/W3C-SVG-1.1/animate-elem-05-t.svg svg/W3C-SVG-1.1/animate-elem-02-t.svg svg/W3C-SVG-1.1/animate-elem-03-t.svg svg/W3C-SVG-1.1/animate-elem-06-t.svg svg/W3C-SVG-1.1/animate-elem-13-t.svg svg/W3C-SVG-1.1/animate-elem-10-t.svg
Jessie Berlin
Comment 11
2011-09-19 10:02:43 PDT
(In reply to
comment #10
)
> (From update of
attachment 107759
[details]
) >
Attachment 107759
[details]
did not pass chromium-ews (chromium-xvfb): > Output:
http://queues.webkit.org/results/9725662
> > New failing tests: > svg/W3C-SVG-1.1/animate-elem-12-t.svg > svg/W3C-SVG-1.1/animate-elem-04-t.svg > svg/W3C-SVG-1.1/animate-elem-07-t.svg > platform/chromium/fast/text/text-stroke-with-border.html > svg/W3C-SVG-1.1/animate-elem-08-t.svg > svg/W3C-SVG-1.1/animate-elem-09-t.svg > printing/return-from-printing-mode.html > platform/mac-snowleopard/platform/mac/fast/text/international/Geeza-Pro-vertical-metrics-adjustment.html > svg/W3C-SVG-1.1/animate-elem-11-t.svg > svg/W3C-SVG-1.1/animate-elem-05-t.svg > svg/W3C-SVG-1.1/animate-elem-02-t.svg > svg/W3C-SVG-1.1/animate-elem-03-t.svg > svg/W3C-SVG-1.1/animate-elem-06-t.svg > svg/W3C-SVG-1.1/animate-elem-13-t.svg > svg/W3C-SVG-1.1/animate-elem-10-t.svg
Not sure why these tests would be failing after my change - they don’t appear to deal with private browsing or visited links. Notably, they also didn’t fail on the chromium-mac ews ...
Jessie Berlin
Comment 12
2011-09-19 10:03:54 PDT
(In reply to
comment #11
)
> (In reply to
comment #10
) > > (From update of
attachment 107759
[details]
[details]) > >
Attachment 107759
[details]
[details] did not pass chromium-ews (chromium-xvfb): > > Output:
http://queues.webkit.org/results/9725662
> > > > New failing tests: > > svg/W3C-SVG-1.1/animate-elem-12-t.svg > > svg/W3C-SVG-1.1/animate-elem-04-t.svg > > svg/W3C-SVG-1.1/animate-elem-07-t.svg > > platform/chromium/fast/text/text-stroke-with-border.html > > svg/W3C-SVG-1.1/animate-elem-08-t.svg > > svg/W3C-SVG-1.1/animate-elem-09-t.svg > > printing/return-from-printing-mode.html > > platform/mac-snowleopard/platform/mac/fast/text/international/Geeza-Pro-vertical-metrics-adjustment.html > > svg/W3C-SVG-1.1/animate-elem-11-t.svg > > svg/W3C-SVG-1.1/animate-elem-05-t.svg > > svg/W3C-SVG-1.1/animate-elem-02-t.svg > > svg/W3C-SVG-1.1/animate-elem-03-t.svg > > svg/W3C-SVG-1.1/animate-elem-06-t.svg > > svg/W3C-SVG-1.1/animate-elem-13-t.svg > > svg/W3C-SVG-1.1/animate-elem-10-t.svg > > Not sure why these tests would be failing after my change - they don’t appear to deal with private browsing or visited links. Notably, they also didn’t fail on the chromium-mac ews ...
AND they didn’t fail with the first version of this patch. The only difference between the first and second version is that the second version has a test.
Jessie Berlin
Comment 13
2011-09-19 11:00:16 PDT
Comment on
attachment 107759
[details]
Patch (take 2 - with API test) Thanks Brady! Committed in
http://trac.webkit.org/changeset/95440
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug