[FIX]Found text is no longer highlighted within INPUT text fields and TEXTAREAs

RESOLVED FIXED

Status

SeaMonkey
Search
--
minor
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: Darren, Assigned: bz)

Tracking

({regression, verified1.8.1.12})

1.8 Branch
regression, verified1.8.1.12
Bug Flags:
blocking1.8.1.12 -
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en; rv:1.8.1.10pre) Gecko/20071108 Camino/1.5.4pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en; rv:1.8.1.10pre) Gecko/20071108 Camino/1.5.4pre

When searching for a string within a page that has form fields or textarea fields with text within the fields, Camino no longer highlights the found text string. Cursor appears to move, but visually, there's very little indication that the string has been found within the field.

Reproducible: Always

Steps to Reproduce:
1. Visit a page with a TEXTAREA or INPUT field with some text prefilled.
2. Search for a known string within the TEXTAREA or INPUT field.
3. Camino will fail to hilite the found text.
Actual Results:  
TEXTAREA scrolls and cursor is moved, but text string is not color highlighted

Expected Results:  
Text should be highlighted.

This feature was previously working around version 1.5.1.

Comment 1

10 years ago
This broke somewhere between 20071003 and 20071004, probably due to bug 388784.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Confirming.

This broke between 2007-10-03 00:00 and 2007-10-04 01:00 Camino branch builds.

Checkins: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-10-03+00%3A00&maxdate=2007-10-04+02%3A00&cvsroot=%2Fcvsroot

Lots of fun-looking bugs in there; bug 388784 and bug 393770 seem vaguely possible. If I knew anything about code, I'd lean toward Smaug's bug over Mats's bug.

I tried to see if this was broken in SeaMonkey 1.1.x also (since we're two apps that don't use Firefox TAF), but I can't get any SeaMonkey branch builds to run properly on 10.3.9 or 10.5 :/
Component: Accessibility → HTML Form Controls
Flags: blocking1.8.1.10?
Keywords: regression
QA Contact: accessibility → form.controls
Version: unspecified → 1.8 Branch
(In reply to comment #2)
> I tried to see if this was broken in SeaMonkey 1.1.x also (since we're two apps
> that don't use Firefox TAF), but I can't get any SeaMonkey branch builds to run
> properly on 10.3.9 or 10.5 :/

Nevermind; apparently SM on the branch can't run correctly from the .dmg.  SeaMonkey's Find (Cmd-F) shows this lack-of-highlight bug as well.
Blocks: 388784
Component: HTML Form Controls → Layout: Form Controls
Product: Camino → Core
QA Contact: form.controls → layout.form-controls
This happens in SeaMonkey 1.1.6 on Windows as well.
OS: Mac OS X → All
Hardware: Macintosh → All
So this is not a problem in Firefox on branch?
No, it's only a problem for uses of the XPFE find-as-you-type implementation, I believe. I can't reproduce it in Firefox 2.0.0.9.
s/find-as-you-type/standard non-find-as-you-type find/

(On the branch, XPFE FAYT places the cursor but doesn't highlight; that's another bug that was filed long ago, stolen pre-Firefox 2 to use as a place to stash the Firefox fix for FAYT highlight, and closed, but it's not this bug.  This bug is about the standard Find impl that's not based on FAYT/TAF code.)
Hmm.  Anyone happen to know where in the find code the highlighting's supposed to be happening?

Comment 9

10 years ago
Does this happen with pages which don't have <label> connected to <input> or <textarea>? Or does XPFE n-FAYT-f use <label> for something?
Bug 388784 is only about focusing <label>.
Created attachment 288033 [details]
<label>-less testcase

I can reproduce the failure using this very simple testcase :(
This is absolutely a regression from bug 393770.  That patch changed what IsNativeAnonymous() returns for the textnodes inside inputs, and the find code depended on the old value.
Blocks: 393770
No longer blocks: 388784
Component: Layout: Form Controls → Search
QA Contact: layout.form-controls → search
Created attachment 288113 [details] [diff] [review]
Fix

This seems to fix the problem for me.  On trunk, it looks like the use of nsIFrame::GetSelectionController is what saves the day... In fact, on trunk this code should just not walk out of anonymous content perhaps.  If we ever DO return true for IsNativeAnonymous on those textnodes on trunk, it will break.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #288113 - Flags: superreview?(mats.palmgren)
Attachment #288113 - Flags: review?(mats.palmgren)
Created attachment 288114 [details] [diff] [review]
Trunk patch as described
Attachment #288114 - Flags: superreview?(mats.palmgren)
Attachment #288114 - Flags: review?(mats.palmgren)
Summary: Found text is no longer highlighted within INPUT text fields and TEXTAREAs → [FIX]Found text is no longer highlighted within INPUT text fields and TEXTAREAs
1.8.1.10 is being rushed, this will have to catch the next train after some trunk baking of the patch
Flags: blocking1.8.1.10? → blocking1.8.1.11?
Daniel, there isn't going to be any trunk baking.  This is a branch-only regression.  The trunk patch just adjusts the code to be a little less regression-prone in the future, but doesn't change any behavior.

It might still be good to get some branch baking, of course, so I agree that we'll have to make do with 1.8.1.11.
bz's patch does fix the issue for us in Camino (in case anyone was waiting for that info).
Comment on attachment 288113 [details] [diff] [review]
Fix

>-    if (!content->IsNativeAnonymous()) {
>+    if (!IsNativeAnonymous(content)) {

Maybe we should us a different name to reduce the risk of mistaking
one for the other? (IsNativeAnonymousDescendent(content) perhaps?)

Looks fine otherwise.  r+sr=mats
Attachment #288113 - Flags: superreview?(mats.palmgren)
Attachment #288113 - Flags: superreview+
Attachment #288113 - Flags: review?(mats.palmgren)
Attachment #288113 - Flags: review+
Comment on attachment 288114 [details] [diff] [review]
Trunk patch as described

ditto
Attachment #288114 - Flags: superreview?(mats.palmgren)
Attachment #288114 - Flags: superreview+
Attachment #288114 - Flags: review?(mats.palmgren)
Attachment #288114 - Flags: review+
Comment on attachment 288113 [details] [diff] [review]
Fix

Requesting branch approvals.  This is quite safe, but I would probably put it in .11 if .10 is anywhere close to being done.
Attachment #288113 - Flags: approval1.8.1.11?
Attachment #288113 - Flags: approval1.8.1.10?
Comment on attachment 288114 [details] [diff] [review]
Trunk patch as described

Requesting trunk approval.  This is bulletproofing/cleanup that should prevent this bug regressing on trunk if our anonymous content behavior is tweaked slightly.
Attachment #288114 - Flags: approval1.9?

Updated

10 years ago
Attachment #288114 - Flags: approval1.9? → approval1.9+
Checked in trunk patch, with s/IsNativeAnonymous/IsInNativeAnonymousSubtree/g.  Leaving bug open, since this is filed as a 1.8 branch bug.

It would be nice to have tests here; I just don't know our Find APIs well enough to tell which ones to use for said test.  Can someone help?
Flags: in-testsuite?
Attachment #288113 - Flags: approval1.8.1.10?
Duplicate of this bug: 407432
Not blocking because it won't stop us shipping Firefox, but will consider the patch for approval
Flags: blocking1.8.1.12? → blocking1.8.1.12-
Comment on attachment 288113 [details] [diff] [review]
Fix

approved for 1.8.1.12, a=dveditz for release-drivers
Attachment #288113 - Flags: approval1.8.1.12? → approval1.8.1.12+
Fixed on branch.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: fixed1.8.1.12
Resolution: --- → FIXED
Sam, can you verify the fix using a Camino build, please?
Verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en; rv:1.8.1.12pre) Gecko/20080116 Camino/1.6b1 (like Firefox/2.0.0.12pre)
Keywords: fixed1.8.1.12 → verified1.8.1.12
(Reporter)

Comment 28

10 years ago
With ONE CAVEAT, this bug is FIXED in the latest nitely I have:
Version 2008011701 (1.5.5pre)

Command-F Find works as expected. Thanks!
/-key "Find as you type" does NOT hilite text.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to comment #28)
> With ONE CAVEAT, this bug is FIXED in the latest nitely I have:
> Version 2008011701 (1.5.5pre)
> 
> Command-F Find works as expected. Thanks!
> /-key "Find as you type" does NOT hilite text.

Darren, "find as you type" has never worked correctly; see comment 7.  This bug was explicitly about Command-F.

Setting this back to FIXED.
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED
FWIW, I tracked down the bug I mentioned in comment 7 again.  It's bug 189039 and it stayed in Core:Keyboard:Find-as-you-type (the component for xpfe fayt), but the patch only touched toolkit/components/typeaheadfind/ (and some other toolkit code).

Comment 31

10 years ago
Confirmed in nightly. I just wanted to say THANK YOU.
(bug id=407432 was mine.  I NEED this functionality!)
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.