Closed Bug 1293618 Opened 8 years ago Closed 6 years ago

"Clear History from Private Data" completely broken

Categories

(SeaMonkey :: Bookmarks & History, defect)

SeaMonkey 2.45 Branch
Unspecified
All
defect
Not set
major

Tracking

(seamonkey2.44 unaffected, seamonkey2.45 fixed, seamonkey2.46 fixed, seamonkey2.47 fixed, seamonkey2.48 fixed)

RESOLVED FIXED
seamonkey2.48
Tracking Status
seamonkey2.44 --- unaffected
seamonkey2.45 --- fixed
seamonkey2.46 --- fixed
seamonkey2.47 --- fixed
seamonkey2.48 --- fixed

People

(Reporter: chokito, Assigned: frg, NeedInfo)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(4 files, 2 obsolete files)

Attached file SM245.pdf
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:48.0) Gecko/20100101 Firefox/48.0 SeaMonkey/2.45
Build ID: 20160804123236

Steps to reproduce:

Steps for reproducing
1. http://seamonkey.callek.net/
2. Click contrib/
3. Text "Parent Directory" shows "visited"
4. Tools > Clear Private Data
5. Click "Reload current page" or "F5"


Actual results:

6. Text "Parent Directory" shows always "visited"


Expected results:

6. Text "Parent Directory" should shows "not visited"
User agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0 SeaMonkey/2.45
Build identifier: 20160804042443
and
User agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:48.0) Gecko/20100101 Firefox/48.0 SeaMonkey/2.45
Build identifier: 20160804123236
-
User agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0 SeaMonkey/2.47a2
Build identifier: 20160808025729
-
-The same problem
REPRODUCIBLE with official en-US SeaMonkey 2.48a1  (NT 6.1; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0 Build 20160804000913  (Default Classic Theme) on German WIN7 64bit:

1. In Browser: Menu ˋTools → Switch User Profile → Create New file and Use itˊ
   » New Profile with SeaMonkey start page opens
2. Visit several Pages by link → Check History
   » Various History entries visible
3. Menu ˋEdit → Preferences → Privacy and Security → Private Data → 
   Check everything → [ok]ˊ
4. Menu ˋTools → Clear Private Dataˊ
   » Dialog with all items checked appears
5. ˋ[Clear Private Data]ˊ
   Expected: History empty
   Actual: no changes in history

a) Also Affected: Menu ˋEdit → Preferences → Privacy and Security → 
   Private Data → [Clear Now]ˊ
b) Also Affected:  ˋAlways clear my private data when I close SeaMonkeyˊ
c) Not yet tested whether other private data (cookies, ...) also affected.
d) Was still ok with  en-US SeaMonkey 2.40 final Mozilla/5.0 (Windows NT 6.1;
   WOW64; rv:43.0) from official download page, Gecko/20100101 Firefox/ 43.0
   Build 20160120202951, (Default Classic Theme) on German WIN7 64bit
d1) Version of appearance not yet tested.

@Reporter:
can you please test for other private data (cookies, ...)?
Severity: normal → major
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: visited links will not deleted → "Clear History from Private Data" completely broken
e) Currently only tested for WIN7
OS: Unspecified → Windows 7
f) 'Clear History' works fine with FF 51.0a1 (2016-08-07)
looks like /suite/modules/Sanitizer.jsm needs an overhaul.
In Data Manager will Website and Cookies removed when Clear Private Data clicked, with SM 2.45.
With SM 2.44 was this problem not.
(In reply to chokito from comment #6)
> In Data Manager will Website and Cookies removed when Clear Private Data

Indeed, other private data - I tested cookies - will be deleted correctly:

11. In Browser (2.48) : Menu ˋTools → Switch User Profile → Create New file 
   and Use itˊ
   » New Profile with SeaMonkey start page opens
12. Visit <http://www.kaninchenforum.de/> → via ˋTools → Cookie Managerˊ check
   Cookies
   » Various entries visible for "kaninchenforum.de"
13. Menu ˋEdit → Preferences → Privacy and Security → Private Data → 
   Check everything → [ok]ˊ
14. Leave "kaninchenforum.de" → Quit SeaMonkey → Launch SeaMonkey
15. Via ˋTools → Cookie Managerˊ check Cookies
   Expected = Actual: no entries for "kaninchenforum.de"
Was still ok with unofficial German SeaMonkey 2.44  Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:47.0)  Gecko/20100101 Firefox/47.0 Build 20160608085536  (Default Classic Theme) on German WIN7 64bit:

My steps 5 and 15 work fine, private data will be deleted.
The same problem exists in aurora and central-trunk.
-
User agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0 SeaMonkey/2.47a2
Build identifier: 20160810024233
and
User agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0 SeaMonkey/2.48a1
Build identifier: 20160810001813
The same problem exists in the versions from Adrian Kalla.
-
User agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0 SeaMonkey/2.45
Build identifier: 20160811123928
and
User agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:48.0) Gecko/20100101 Firefox/48.0 SeaMonkey/2.45
Build identifier: 20160811081033
Depends on: 1254923
See Also: → 1124185
Simple replace with the new function. Tested with 2.48a1.

[Approval Request Comment]
Regression caused by (bug #): 1254923
User impact if declined: ONe click history clearing is not possible.
Testing completed (on m-c, etc.): c-c
Risk to taking this patch (and alternatives if risky): None its broken already.
String changes made by this patch:

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky):
String changes made by this patch:

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky):
String changes made by this patch:

Simple replace. Tested in c-c.
Assignee: nobody → frgrahl
Status: NEW → ASSIGNED
Attachment #8780349 - Flags: review?(philip.chee)
Attachment #8780349 - Flags: review?(iann_bugzilla)
Attachment #8780349 - Flags: approval-comm-release?
Attachment #8780349 - Flags: approval-comm-beta?
Attachment #8780349 - Flags: approval-comm-aurora?
Patch for Thunderbird. Untested. Please check.
Attachment #8780350 - Flags: review?(jorgk)
Assignee: frgrahl → nobody
Status: ASSIGNED → NEW
Tests need to wait till tomorrow. Too late already.
Assignee: nobody → frgrahl
Status: NEW → ASSIGNED
Comment on attachment 8780350 [details] [diff] [review]
1293618-historeyclear-tb.patch

Seems reasonable. Thanks. Do you know how to view the history in TB?
>> Seems reasonable. Thanks. Do you know how to view the history in TB?

Not using TB and Seamonkey only occasionally for mail but it looks like the sanitizer dialog in TB is located under the Tools menu in 'Clear Recent History…'. I found only this one occurrence. 

TB unlike Seamonkey also uses removeVisitsByTimeframe. This one is also marked deprecated and might get removed in a follow up bug from toolkit.
(In reply to Frank-Rainer Grahl from comment #15)
> Not using TB and Seamonkey only occasionally for mail but it looks like the
> sanitizer dialog in TB is located under the Tools menu in 'Clear Recent
> History…'. I found only this one occurrence.
Yes, I found that, too, but I need to check before and after clearing. So I need to list the history somehow, or at least the cookies (via Options, Privacy).

In fact, I just did and had a cookie. Then I went to "Clear Recent History" and when I checked again, the cookie was gone using a Daily without your patch. Oh I see in sanitize.js that cookies and history (as in: visited pages) are treated differently. So I'd still have to see the history before and after clearing it.
>> So I'd still have to see the history before and after clearing it.

Link color of previously vistited urls in stored emails should change when you clear the history. Cookies was/is indeed not affected.
Comment on attachment 8780350 [details] [diff] [review]
1293618-historeyclear-tb.patch

(In reply to Frank-Rainer Grahl from comment #17)
> Link color of previously visited urls in stored emails should change when
> you clear the history.
Thanks, it does!
r=jorgk.
Attachment #8780350 - Flags: review?(jorgk) → review+
Jorg,

Tree is closed. Will check it in when its open again. Do you want the patch in c-r to c-a? If yes let me know who can approve it. clokep?
Flags: needinfo?(jorgk)
OS: Windows 7 → All
Comment on attachment 8780350 [details] [diff] [review]
1293618-historeyclear-tb.patch

I handle Aurora and Beta approvals and uplifts for TB. Since this is in SM, I can't approve. I can land it however ;-)

[Approval Request Comment]
Regression caused by (bug #): Bug 1254923 (landed on Mozilla48)
User impact if declined: Can't clear history in TB.
Testing completed (on m-c, etc.): Manual.
Risk to taking this patch (and alternatives if risky):
Not risky.
Flags: needinfo?(jorgk)
Attachment #8780350 - Flags: approval-comm-beta?
Attachment #8780350 - Flags: approval-comm-aurora?
>>
I can't approve.
<<

Me neither. Personally I would consider it approved. I am sure IanN or Ratty will do it when they are online again.
Need to test this likely tomorrow. Not sure if the patch for the tests is ok.
Attachment #8780568 - Flags: review?(philip.chee)
Attachment #8780568 - Flags: review?(iann_bugzilla)
Comment on attachment 8780349 [details] [diff] [review]
1293618-historyclear-suite.patch

r/a=me
Attachment #8780349 - Flags: review?(iann_bugzilla)
Attachment #8780349 - Flags: review+
Attachment #8780349 - Flags: approval-comm-release?
Attachment #8780349 - Flags: approval-comm-release+
Attachment #8780349 - Flags: approval-comm-beta?
Attachment #8780349 - Flags: approval-comm-beta+
Attachment #8780349 - Flags: approval-comm-aurora?
Attachment #8780349 - Flags: approval-comm-aurora+
Ian, can you please set approval-comm-aurora+/approval-comm-beta+ on the TB patch as well.
Attachment #8780350 - Flags: approval-comm-beta?
Attachment #8780350 - Flags: approval-comm-beta+
Attachment #8780350 - Flags: approval-comm-aurora?
Attachment #8780350 - Flags: approval-comm-aurora+
Attachment #8780349 - Flags: review?(philip.chee)
Landed for TB:
TB 51: https://hg.mozilla.org/comm-central/rev/4ee27b8eb653
TB 50 (Aurora): https://hg.mozilla.org/releases/comm-aurora/rev/8ecda04edc50
TB 49 (Beta): https://hg.mozilla.org/releases/comm-beta/rev/485ee4e494fa
I've landed these so they don't get forgotten since in this SM bug we have no TB tracking.
>> I've landed these so they don't get forgotten since in this SM bug we have no TB tracking.

Wouldn't have needed to worry. Didn't put them in because tree is closed and I thought you wanted them first in c-c.

FRG
I took the easy way and just ported the existing three Firefox tests. Local Mochitest tests seem to be completly broken and I was unable to get this working :(

Just try:
mozilla/mach mochitest -f chrome suite/common/places/tests/chrome/test_bug549192.xul

You need to copy the suite mozconfig into the mozilla dir because of bug 1164003
Attachment #8780568 - Attachment is obsolete: true
Attachment #8780568 - Flags: review?(philip.chee)
Attachment #8780568 - Flags: review?(iann_bugzilla)
Attachment #8780837 - Flags: review?(iann_bugzilla)
I screwed the previous V2 patch up. Sorry.
Attachment #8780837 - Attachment is obsolete: true
Attachment #8780837 - Flags: review?(iann_bugzilla)
Attachment #8780838 - Flags: review?(iann_bugzilla)
(In reply to Frank-Rainer Grahl from comment #28)
> I took the easy way and just ported the existing three Firefox tests. Local
> Mochitest tests seem to be completly broken and I was unable to get this
> working :(

I have a patch for that in Bug 1268970 but it was r- [help-wanted]
Comment on attachment 8780838 [details] [diff] [review]
1293618-historyclear-suite-tests-V2a.patch

You will need to include the changes to head.js
Attachment #8780838 - Flags: review?(iann_bugzilla) → review-
Flags: needinfo?(frgrahl)
Flags: needinfo?(philip.chee)
>> You will need to include the changes to head.js

I know and will not forget. Bug itself is fixed or what do you mean?
Flags: needinfo?(frgrahl)
No longer  reproducible with Server-Installation of  unofficial (by FRG) en-US SeaMonkey 2.49a2  (NT 6.1; WOW64; rv:52.0) Gecko/20100101 Firefox/52.0 Build 20161118170316  (Default Classic Theme) on German WIN7 64bit:
I did tests concerning Original report, Comment 2, Comment 2, Comment 7
TM for easing queries.

Can we close this one?
Target Milestone: --- → seamonkey2.45
Rainer, the target milestone is always indicating when this was checked in on /trunk/ and thus 2.48 in this case (despite landing on c-a/c-b/c-r for earlier versions, which is the reason for the separate set of tracking flags).

Again though, why is this still open? For the tests?
Flags: needinfo?(frgrahl)
Target Milestone: seamonkey2.45 → seamonkey2.48
>> Again though, why is this still open? For the tests?

Yes. I still have them on the radar but on the backburner till I find some time or run out of other non test bugs to fix :)
Flags: needinfo?(frgrahl)
> the target milestone is always indicating when this was checked in on /trunk/
Where is a source for this, and for what is this information in TM field useful?
(In reply to rsx11m from comment #35)
No! For ages I have been doing QA for open source and open contents projects, most projects use BZ as bug tracking system, and in none of those projects TM has been used that way. Unfortunately also our bugzilla.mozilla.org help is misleading, it's correct that in some projects developers publish for what version they plan a fix, but in all projects I know finally TM shows the earliest Version where the fix has become integrated. So - together with VERSION - TM indicates the range of version affected by a bug. This information is important for QA (deciding whether bugs with similar symptoms are DUPs) and for users (New features already available?). Regressions which are affecting me in a particular version?)
For Mozilla/Seamonkey please also see 
<https://wiki.mozilla.org/index.php?title=SeaMonkey/Bugzilla-FAQ&oldid=1155843#Target_Milestone>
<https://developer.mozilla.org/en-US/docs/Mozilla/QA/A_Bugs_Life?document_saved=true#resolved>
<https://developer.mozilla.org/en-US/docs/Mozilla/QA/Bug_Verification_Day>

So I revert your changes, because correct TM information is required for SeaMonkey 2.46 release notes.
.
Target Milestone: seamonkey2.48 → seamonkey2.45
(In reply to Rainer Bielefeld from comment #38)
> So I revert your changes, because correct TM information is required for
> SeaMonkey 2.46 release notes.

... which screws it up for the Wiki-page Status Meetings queries. I don't see anything in those pages you cite (except for the example you've added yourself to document your interpretation) relating to what the target milestone is actually set to. My information comes from Jens and has been practice over several years now for both Thunderbird and SeaMonkey to keep track of changes as they land on the current development repository (notwithstanding any uplifting to the branches later, which can be queried by the status flags).

So, please do not change unilaterally the semantics of the fields as they are used.
If you want to change their usage, come to the status meetings and propose them there.
Flags: needinfo?(iann_bugzilla)
So, as a current example, see bug 1328686 which was fixed for both Firefox 53 on mozilla-central and Firefox 52 on mozilla-aurora, yet the TM is retained as Firefox 53; or bug 1329941, which also landed on mozilla-beta, also has the TM set for Firefox 53. Just follow more examples from https://hg.mozilla.org/releases/mozilla-aurora or https://hg.mozilla.org/releases/mozilla-beta to see yourself that this is the way how it's currently supposed to be done.

Whether or not it's the "right" way, it nevertheless has to be consistent for exactly the reason you are stating (looking up bugs per cycle), which is violated by your edits in the bugs and the documentation pages. I certainly agree that the policy should be explicitly documented.
Target Milestone: seamonkey2.45 → seamonkey2.48
TM should be the version of c-c that the fix lands on, so in this case 2.48
Flags: needinfo?(iann_bugzilla)
Blocks: 1442908
Split the tests to Bug 1442908. This will probably take longer and the issue itself has been resolved.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.