Various find as you type bustage from bug 1133981

RESOLVED FIXED in seamonkey2.39

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: neil, Assigned: neil)

Tracking

({regression})

SeaMonkey 2.37 Branch
seamonkey2.39

SeaMonkey Tracking Flags

(seamonkey2.37 fixed, seamonkey2.38 fixed, seamonkey2.39 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

6.78 KB, patch
iann_bugzilla
: review+
philip.chee
: review+
iann_bugzilla
: approval-comm-beta+
iann_bugzilla
: approval-comm-release+
Details | Diff | Splinter Review
Assignee

Description

4 years ago
Bug 1133981 caused several issues:
* Toggling the autostart preference no longer works
* The findbar does not work in the browser
* The findbar always steals type ahead find in view source
Assignee

Comment 1

4 years ago
Posted patch Proposed patch (obsolete) — Splinter Review
* Fix autostart issue by a) using new variable name in constructor and pref observer b) updating the browser in the pref observer (the parent constructor already updates the browser)
* Rename the usefindbar variable because it's far too long
* Remove obsolete code from destroy method
* Remove obsolete _shouldFastFind method
* Fix browser issue by copying receiveMessage method and removing browser test (since SeaMonkey has one find bar for the entire tabbrowser)
* Fix view source issue by making the receiveMessage method also check that we're using the find bar before passing on the keypress

Note that the browser fix makes it steal type ahead find in the same way as the view source issue but the fix for that fixes the browser again too.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #8638953 - Flags: review?(iann_bugzilla)
Attachment #8638953 - Flags: feedback?(philip.chee)

Updated

4 years ago
Attachment #8638953 - Flags: review?(iann_bugzilla) → review+

Comment 2

4 years ago
Comment on attachment 8638953 [details] [diff] [review]
Proposed patch

Start SeaMonkey go to some page type "/" findbar opens.

Open more tabs. type "/" FAYT message shows up in the status bar for all tabs except the first one.
Attachment #8638953 - Flags: feedback?(philip.chee) → feedback-
Assignee

Comment 3

4 years ago
* Switched to using the window message manager in the tabbrowser so that the findbar receives messages from all tabs
* Provided an override of the _updateBrowserWithState function which uses the window message manager (the function name is different)
Attachment #8638953 - Attachment is obsolete: true
Attachment #8642057 - Flags: review?(philip.chee)
Attachment #8642057 - Flags: review?(iann_bugzilla)

Updated

4 years ago
Attachment #8642057 - Flags: review?(philip.chee) → review+
Assignee

Comment 4

4 years ago
Pushed comm-central changeset 5befa64efa62.
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.39
Version: SeaMonkey 2.35 Branch → SeaMonkey 2.37 Branch
Assignee

Comment 5

4 years ago
Comment on attachment 8642057 [details] [diff] [review]
Updated for review comments

[Approval Request Comment]
Regression caused by (bug #): 1133981
User impact if declined: Erratic Find bar behaviour
Testing completed (on m-c, etc.): Landed on c-c
Risk to taking this patch (and alternatives if risky): Not risky
String changes made by this patch: None
Attachment #8642057 - Flags: approval-comm-beta?
Attachment #8642057 - Flags: approval-comm-aurora?

Comment 6

4 years ago
Comment on attachment 8642057 [details] [diff] [review]
Updated for review comments

[Triage Comment]
a=me for the relevant branches it is needed on.
Attachment #8642057 - Flags: review?(iann_bugzilla)
Attachment #8642057 - Flags: review+
Attachment #8642057 - Flags: approval-comm-release+
Attachment #8642057 - Flags: approval-comm-beta?
Attachment #8642057 - Flags: approval-comm-beta+
Attachment #8642057 - Flags: approval-comm-aurora?
Attachment #8642057 - Flags: approval-comm-aurora+
Assignee

Comment 7

4 years ago
(In reply to Ian Neal from comment #6)
> a=me for the relevant branches it is needed on.

Whoa, I hadn't realised it was uplift time already.
In that case, yeah, I need c-b and c-r.

Comment 8

4 years ago
Comment on attachment 8642057 [details] [diff] [review]
Updated for review comments

Removing the unneeded c-a a=
Attachment #8642057 - Flags: approval-comm-aurora+
You need to log in before you can comment on or make changes to this bug.