412646 - Dot under caret in find bar

Status: VERIFIED FIXED

(Core :: Layout, defect, P2)

Description

[Ria Klaassen (not reading all bugmail)]

Between two blinks of the caret I see a dot in the find bar's input field.

When I remove the code 
#FindToolbar {
 overflow-x: hidden;
the problem disappears.

I first noticed the problem because in spite of the userChrome.css code
#FindToolbar { -moz-box-direction: reverse; }
the find bar appeared on an unusual place.

Comment 1

[Ria Klaassen (not reading all bugmail)]

Attachment: screen shot

Comment 2

[Steve England [:stevee]]

No Dot: 20080115_1726_firefox-3.0b3pre.en-US.win32
Dot: 20080115_1957_firefox-3.0b3pre.en-US.win32

Checkins to module PhoenixTinderbox between 2008-01-15 17:26 and 2008-01-15 19:56 : 
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1200446760&maxdate=1200455819

Comment 3

[Reed Loden [:reed]]

I see this on Linux, too.

Comment 4

[Ria Klaassen (not reading all bugmail)]

The dot appeared for the first time in June 2006; it was a 1 pixel dot.
It became a two pixels dot in Feb 2007 when bug 177805 was checked in.

Comment 5

[Ria Klaassen (not reading all bugmail)]

Comment 4 was tested including overflow-x: hidden. If that code was used it would have appeared for the first time in June 2006. Don't know if it is useful to know when exactly.

Comment 6

[Dão Gottwald [:dao]]

In case there's no obvious fix, we should back out the find bar part of bug 312247.

Comment 7

[Masahiro YAMADA]

(copyed from Bug 311247 Comment 38)
I think dot in find bar text box is caused by incorrect height of blinking
caret.
When caret is drawn by reflow, window expose, moving caret by text input, its
height is correct.
But when automatic blinking, its height is not valid (top is valid. But bottom
is not valid).

Comment 8

[Ria Klaassen (not reading all bugmail)]

Both problems from comment 0 can temporarily be corrected with a userChrome.css with 

#FindToolbar {
  overflow-x: visible !important;
}

Is problem 2 a separate bug?

Comment 9

[Martijn Wargers (dead)]

Attachment: testcase

You should see some text appear in the text field.

Comment 10

[Martijn Wargers (dead)]

Attachment: workaround for the issue

Comment 13

[Daniel Holbert [:dholbert]]

Note that this also seems to affect the "close" button at the left end on the find bar -- when I mouseover it, its lower border is missing.
  (unless the cursor happens to be hovering over it when the find-bar appears, in which case the lower border sticks around when it shouldn't be there.)

Comment 14

[Masahiro YAMADA]

Martijn's workaround can fix Bug 414708 too.

Comment 15

[Teune van Steeg]

Note that with Martijn's workaround, the overflow-x: hidden; declaration can be removed. It seems that that declaration is only necessary because of the align="center" part on the findbar.

Comment 16

[Martijn Wargers (dead)]

Oh, that is useful information. Thanks, Teune.
In that case, when that overflow-x: hidden; rule can be removed, then my "workaround for the issue" patch might even be preferred. (although that doesn't mean this should be happening at all)

This might be related to bug 412646, perhaps.

Comment 18

[Daniel Holbert [:dholbert]]

(In reply to comment #16)
> This might be related to bug 412646, perhaps.

I think you mean bug 413027.   (this bug here is bug 412646 :))

Comment 19

[Martijn Wargers (dead)]

Attachment: testcase - overflow:auto

(In reply to comment #18)
> I think you mean bug 413027.   (this bug here is bug 412646 :))

Oops, yes, that's what I meant.

This is a testcase that tests performance of overflow: auto boxes.

trunk:
scrolltest: 5904ms
toggledisplaytest: 228913ms

Just for comparison, I also tested branch
branch: 
scrolltest: 7047ms
toggledisplaytest: 267140ms

Comment 20

[Martijn Wargers (dead)]

Attachment: testcase - two boxes no overflow: auto

This testcase uses two boxes instead of an overflow: auto box, it seems much faster.

trunk:
scrolltest: 4055ms
toggledisplaytest: 6922ms

branch:
scrolltest: 4218ms
toggledisplaytest: 8344ms

Comment 21

[Martijn Wargers (dead)]

Attachment: patch

This patch doesn't seem to regress bug 312247.
The pinstripe changes look strange. It seems to me those css rules could be made simpler, but I'm afraid I can't really test those pinstripe changes at all.
So anyone who could help with that, would be appreciated.

Comment 22

[Dão Gottwald [:dao]]

Comment on attachment 302817 [details] [diff] [review]
patch

toolkit/themes/winstripe/global/findBar.css
> }
>-
> /* find-next button */
> 
> .findbar-find-next {

That line shouldn't be removed.

toolkit/themes/pinstripe/global/findBar.css
>-findbar > toolbarbutton:not([disabled]):hover:active,
>-findbar > hbox > toolbarbutton:not([disabled]):hover:active {
>+findbar > hbox > toolbarbutton:not([disabled]):hover:active,
>+findbar > hbox > hbox > toolbarbutton:not([disabled]):hover:active {
>   background-image: url("chrome://global/skin/icons/white-gray-gradient-active.gif");
> }

Please add a class name to the new hbox, and use the class name of the existing hbox (find-field-container). After that, you can drop "findbar >" here and elsewhere.

Looks good otherwise -- please request review from a toolkit & browser peer.

Comment 24

[Gary Johnson]

Attachment: TestCase h2 inside marquee displayed incorrectly

3.0b3.  regression, an H2 is not displayed correctly inside a marquee.

Comment 28

[Kai de Leeuw]

Isn't this serious enough to constitute a blocker for 1.9?

Comment 31

[Mike Schroepfer]

Given the number of dups let's get this fixed...

Comment 32

[Mike Beltzner [:beltzner, not reading bugmail]]

--> P2, not blocking beta 5 on this, will block final.

Comment 33

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Martijn, is this actually a duplicate, or at least dependent on, bug 413027?

Comment 34

[Martijn Wargers (dead)]

Yes, I think this is basically a duplicate of bug 413027. However, this can be fixed by a workaround (the patch I attached, which is probably a better way of doing anyway).
Sorry for the delay here, I'll update the patch later today, hopefully.

Comment 35

[TimoT]

This could be just my build but at least for me the findbar.xml change breaks the 'find as you type' feature. With the patch starting typing no longer opens the quick find bar.

Comment 36

[Martijn Wargers (dead)]

Attachment: patchv2

Sorry for the delay

Comment 37

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 312096 [details] [diff] [review]
patchv2

This looks fine, but need some small changes:
- no need to move the "findbar" rules to ".findbar-container"
- need to update the "findbar[flash="true"]  > .find-field-container" selector to take into account the new hbox, for both winstripe and gnomestripe

Comment 41

[Mike Schroepfer]

Martijn you have an updated patch here?

Comment 42

[Mike Schroepfer]

This is pretty minor visual artifact - I think at this point we should move this bug to .next... 

Comment 43

[José Jeria]

Attachment: Not just a dot, but a fat black underline

This might only be a dot in Windows. But on Mac OS X you actually see a very fat black underline on the text you typed in.

This fat black underline makes it difficult to see what you just typed.

This is a highly visible regression that already have eleven duplicates. Please reconsider the blocking status.

Comment 44

[Simon Morgan]

Same under Linux, at least for me.

Comment 45

[Karel Koubek]

"This is pretty minor visual artifact" - maybe for users that don't search much. But for me, doing searches practically on every second visited page, this is quite serious. FF3, which looks otherwise as a very polished product is now viewed as this old unbreakable "cheapo FOSS product" myth (I haven't seen such behavior in any other SW product I regularly use, be it FOSS or commercial).

Moreover, this is a regression from previous FF versions, so it should be high on the list. If it were there from the beginning I would get used to it, but this way it sheds some bad light on FF3...

So please reconsider. And sorry for spamming this bug.

Comment 46

[Wayne Woods]

I really hate to add a "me too", but even after many weeks of getting used to this bug, it still catches me out. I keep thinking I've accidentally typed a period in the search and still often habitually backspace to try to get rid of it, before I remember.

Comment 47

[Reed Loden [:reed]]

Would like drivers to reconsider, as we have an almost-finished patch for an issue that while not being harmful to the user is a very bad user experience for a feature that is very widely and frequently used.

Comment 48

[Axel Howind]

Sorry, but for me it's not only a minor visual artifact, but the complete lower part of some letters is missing: the letter 'y' cannot be distinguished from the letter 'v', 'g' looks like 'o', and '_' (underline) is not displayed at all.

I filed a separate bug 426775 for this, but it was marked as a duplicate of this one.

Comment 49

[Damon Sicore (:damons)]

OK, blocking.  Going to finish this Martijn?  Need a reviewed patch, ASAP if we're going to get this in.  I reserve the right to change my mind.  :)

Comment 50

[Kai Liu]

Attachment: patchv3

New patch based on Martijn's, incorporating Gavin's comments.
* Moved ".findbar-container" rules back to "findbar"
* Fixed the flash=true selector for gnomestrip, pinstripe, and winstripe

This is a pretty annoying issue (under certain circumstances, it's not just a dot), so I hope this doesn't get pushed back to 3.0.1

Comment 51

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 314477 [details] [diff] [review]
patchv3

Thanks!

Comment 52

[Mike Connor [:mconnor]]

Comment on attachment 314477 [details] [diff] [review]
patchv3

a=mconnor on behalf of 1.9 drivers

Comment 53

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/base/content/browser.css 	1.61
mozilla/toolkit/content/widgets/findbar.xml 	1.22
mozilla/toolkit/themes/gnomestripe/global/findBar.css 	1.4
mozilla/toolkit/themes/pinstripe/global/findBar.css 	1.11
mozilla/toolkit/themes/winstripe/global/findBar.css 	1.9

Thanks Martijn and Kai!

Comment 54

[:Gavin Sharp [email: [e-mail]]]

Attachment: bustage fix

I landed this to fix the test_bug288254.xul failure.

Comment 55

[Jim Jeffery not reading bug-mail 1/2/11]

This has broken 'Quick Find' ..  the / or the ' does not bring up Find Bar, you see a slight movement at the bottom of the browser window, but the Bar is not displayed.  Also I see nothing in the Error Console

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008040915 Minefield/3.0pre Firefox/3.0 ID:2008040915 <-Latest hourly

Comment 56

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #55)
> This has broken 'Quick Find' ..  the / or the ' does not bring up Find Bar, you
> see a slight movement at the bottom of the browser window, but the Bar is not
> displayed.  Also I see nothing in the Error Console

That's the bustage that the bustage fix fixes :)

Comment 57

[Marcia Knous [:marcia]]

verified fixed using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008041005 Minefield/3.0pre. I verified by testing with the find bar and comparing it the build from yesterday.

Comment 58

[Peter van der Woude [:Peter6]]

(In reply to comment #56)
> (In reply to comment #55)
> > This has broken 'Quick Find' ..  the / or the ' does not bring up Find Bar, you
> > see a slight movement at the bottom of the browser window, but the Bar is not
> > displayed.  Also I see nothing in the Error Console
> 
> That's the bustage that the bustage fix fixes :)
> 
I filed bug 428351 for that

Comment 59

[cris]

i have
#FindToolbar>* {display: -moz-box !important;}
in my userchrome.css to invoke full featured findbar when doing FAYT, now its not working anymore,  is it related to this patch?

Comment 60

[Ria Klaassen (not reading all bugmail)]

.findbar-container>* {display: -moz-box !important;}
seems to work.

Comment 61

[cris]

(In reply to comment #60)
> .findbar-container>* {display: -moz-box !important;}
> seems to work.
> 

thanks!

Comment 63

[Asaf Romano (gone)]

Comment on attachment 314734 [details] [diff] [review]
bustage fix

r=mano

Comment 68

[Martijn Wargers (dead)]

The core cause of this issue is still around. But I think that is basically bug 413027.