Status

()

Core
Layout
P2
normal
VERIFIED FIXED
18 years ago
18 years ago

People

(Reporter: harishd, Assigned: Chris Waterson)

Tracking

({regression})

Trunk
x86
Linux
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [rtm++], URL)

Attachments

(3 attachments)

(Reporter)

Description

18 years ago
Steps to reproduce:

1) Goto the above url.
2) Watch for the search text field, on the top center, between
   "Netscape Search" select box and "search" button.
3) On your Preferences adjust the variable-width font to be some large number
   ..larger than the previous size. Try 40.

Actual Result: Text field shifted downwards compared to the select box and the
               search button.

Expected Result: Text field should bottom align with the select box..I think
                 Verify with Nac. 4.x

Also, typing on the text field seems to correct the, text field, alignment  (
jar does see this happen ).
(Assignee)

Comment 1

18 years ago
For me to repro, I need to set the "fixed width font" to be large. But then I
see the problem. I tried removing my changes for bug 54005, and this problem
went away. Appears to be related to an assertion in ForgetWordFrame(), cf. bug
56073. I'll investigate; at worst, we can revert 54005 and live with the
problems that it solved.
Status: NEW → ASSIGNED
Keywords: regression
(Reporter)

Comment 2

18 years ago
Forgot to mention.

You should have your "variable-width font" and "fixed-width font" to be of the
same size.
Keywords: regression
(Assignee)

Comment 3

18 years ago
Created attachment 16844 [details]
minimized test case
(Assignee)

Comment 4

18 years ago
So the critical ingredients seem to be a text field 1) that is large enough to
force a horizontal scrollbar 2) following an  
(Assignee)

Comment 5

18 years ago
Simplified repro instructions:

1. Open attached test case
2. Type one character in the text field

Expected behavior: nothing happens (well, besides
   a character appearing in the text field)

Actual behavior: text field jumps to a new location!
(Assignee)

Comment 6

18 years ago
Created attachment 16845 [details] [diff] [review]
fix, check if CanContinueTextRun() when traversing down, too.
(Assignee)

Comment 7

18 years ago
*** Bug 54505 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 8

18 years ago
*** Bug 56073 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 9

18 years ago
Well, don't I just suck. Third time's the charm? :-)

I did cause this "regression" when fixing bug 54005. When I moved the
termination condition to *before* retrieving the next bug, I exposed this
problem: as we traverse down through child frames, we don't check *those* to see
if they CanContinueTextRun().

In this case, what was happening was we'd end up with a frame model like so:

Block(form)(1)@00D5D0AC <
  line 00D5D484: <
    Text(0)@00D5D114 <
      "blah"
    >
  >
  line 00D5D560: <
    nsGfxControlFrame2@00D5D150 <
      GfxScroll(div)(-1)@00D5D26C <
        ScrollPortFrame(div)(-1)@00D5D314 <
          Area(div)(-1)@00D5D220 <
            line 00D5D45C:  <
              Text(0)@00D5D5A0 <
                "a"
              >
            >
          >
        >
      >
    >
  >
>

The logic failed to test nsGfxFormControl2 et. al. to see if /they/ could stop
the text run, so we'd mosey all the way down to the form control's text node and
say, "yeah, that'll continue my happy text run." From there, all kinds of stuff
would get out of whack.

The fix is to test each frame as we descend down through the hierarchy to see if
*any* of them are text run busters.
Keywords: regression, rtm
(Assignee)

Comment 10

18 years ago
Created attachment 16847 [details] [diff] [review]
cleaned up patch to avoid warnings on egcs-1.1.2
(Assignee)

Updated

18 years ago
Target Milestone: --- → M18

Comment 11

18 years ago
r=buster
I think this is worthwhile for rtm, given the unpredictable results from the
error and the moderate risk of the fix.  It will need quite a bit of testing
before it goes in the branch, though.

Comment 12

18 years ago
Requesting a triage on this bug. It's nominated but doesn't have a - or +. Are
we going to fix this for rtm ?
(Assignee)

Comment 13

18 years ago
karnaze, I got an a= from sfraser, could you float this for rtm+? It affects
home.netscape.com, of all places...
Keywords: regression, rtm

Comment 14

18 years ago
Changing the url from peterson@netscape.com to home.netscape.com. Marking rtm+ 
because it is our home page.
Keywords: regression, rtm
Priority: P3 → P2
Whiteboard: [rtm+]

Comment 15

18 years ago
RTM++.
Please land on branch asap.
Thanks,
Jim
Whiteboard: [rtm+] → [rtm++]

Comment 16

18 years ago
*** Bug 46679 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 17

18 years ago
Fix checked in on branch and trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 18

18 years ago
Fixed in the Oct 18th branch build.

Updated

18 years ago
Keywords: vtrunk

Comment 19

18 years ago
Verified Fixed on trunk builds
linux 101808 RedHat 6.2
win32 101804 NT 4
mac 101804 Mac OS9
Setting bug to Verified and removing vtrunk keyword
Status: RESOLVED → VERIFIED
Keywords: vtrunk
You need to log in before you can comment on or make changes to this bug.