Last Comment Bug 815127 - Sanitizer: Clear Private Data should clear search bar and find bar data.
: Sanitizer: Clear Private Data should clear search bar and find bar data.
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.17
Assigned To: Philip Chee
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-26 06:47 PST by Philip Chee
Modified: 2012-11-29 07:33 PST (History)
0 users
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1.0 Proposed fix. (5.70 KB, patch)
2012-11-26 07:01 PST, Philip Chee
neil: review-
Details | Diff | Splinter Review
Patch v1.1 fix nits, and clear the search sidebar. (6.01 KB, patch)
2012-11-28 01:50 PST, Philip Chee
neil: review+
Details | Diff | Splinter Review
Patch v1.2 as checked in. r=Neil (5.79 KB, patch)
2012-11-29 07:32 PST, Philip Chee
philip.chee: review+
Details | Diff | Splinter Review

Description Philip Chee 2012-11-26 06:47:32 PST
Referenced Firefox bugs:
Bug 409624 - Sanitizer should clear findBar data.
Bug 446274 - Clear Private Data should clear search bar, but doesn't
Bug 463474 - Make sanitizer's searchbar clearing code a bit cleaner.
Comment 1 Philip Chee 2012-11-26 07:01:18 PST
Created attachment 685150 [details] [diff] [review]
Patch v1.0 Proposed fix.

TEST_PATH=suite/browser/test/browser_bug409624.js  pymake -C ../objdir-sm/ mochitest-browser-chrome

INFO TEST-START | Shutdown
Browser Chrome Test Summary
        Passed: 4
        Failed: 0
        Todo: 0

TEST_PATH=suite/modules/test/browser_sanitizer.js   pymake -C ../objdir-sm/ mochitest-browser-chrome

INFO TEST-START | Shutdown
Browser Chrome Test Summary
        Passed: 41
        Failed: 4
        Todo: 0

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/modules/test/browser_sanitizer.js | Cache test setup successfully
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/modules/test/browser_sanitizer.js | Offline app cache test setup successfully
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/modules/test/browser_sanitizer.js | Cache test setup successfully for full sanitize
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/modules/test/browser_sanitizer.js | Offline app cache test setup successfully for full sanitize
Comment 2 neil@parkwaycc.co.uk 2012-11-27 09:32:53 PST
Comment on attachment 685150 [details] [diff] [review]
Patch v1.0 Proposed fix.

>+          var searchBar = currentDocument.getElementById("searchbar");
>+          if (isElementVisible(searchBar)) {
View - Show/Hide - (customised toolbar) will make isElementVisible return false, although the textbox remembers its value and undo.

What about the search sidebar?
Comment 3 Philip Chee 2012-11-28 01:50:54 PST
Created attachment 686035 [details] [diff] [review]
Patch v1.1 fix nits, and clear the search sidebar.

>>+          var searchBar = currentDocument.getElementById("searchbar");
>>+          if (isElementVisible(searchBar)) {
> View - Show/Hide - (customised toolbar) will make isElementVisible return false, although the textbox remembers its value and undo.
Fixed.

> What about the search sidebar?
Now clears the search sidebar as well.
Comment 4 neil@parkwaycc.co.uk 2012-11-28 09:09:00 PST
Comment on attachment 686035 [details] [diff] [review]
Patch v1.1 fix nits, and clear the search sidebar.

>+        // Clear undo history of all search and find bars.
Nit: can you do find bars first and keep the search bars together?

>+          var sidebar = currentDocument.querySelector("browser[src*='search-panel.xul']");
This isn't really the right way to do this, see BrowserSearch.searchSidebar

[Hmm, what about sidebars in non-browser windows?]
Comment 5 Philip Chee 2012-11-29 07:32:40 PST
Created attachment 686589 [details] [diff] [review]
Patch v1.2 as checked in. r=Neil

Pushed comm-central changeset 582d55c0c395
http://hg.mozilla.org/comm-central/rev/582d55c0c395

>>+        // Clear undo history of all search and find bars.
> Nit: can you do find bars first and keep the search bars together?
Fixed.

>>+          var sidebar = currentDocument.querySelector("browser[src*='search-panel.xul']");
> This isn't really the right way to do this, see BrowserSearch.searchSidebar
Fixed.

> [Hmm, what about sidebars in non-browser windows?]
I'll leave this to another bug.

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