50833 - nsTextBoxFrame's crop=middle is inverse of what is desired.

Status: VERIFIED FIXED

(Core :: XUL, defect, P3)

Description

[Mike Pinkerton (not reading bugmail)]

- open bookmarks
- find a long url that needs to be truncated because it's too wide for the column

expected:
- should be middle truncated, eg http://www.cnn.com/blah.../blah/foo.html

actual:
- start/end truncated, eg .../blah/blah/...

this is basically useless. i think this is a bug in cropping in titled buttons, 
assigning to evaughan.

Comment 1

[Mike Pinkerton (not reading bugmail)]

makes bookmarks kinda hard to use. nsbeta3

Comment 2

[Peter Trudelle]

nsbeta3-/future, need to make the column wider til beta5 (aka 6.0.1)

Comment 3

[Claudius Gayle]

you understand the column will almost never be wide enough and typically one cannot literally make head nor tail of the url?
I understand it's crunch time, but I'm giving it one more try.

Comment 4

[Peter Trudelle]

nsbeta3-

Comment 5

[Mike Pinkerton (not reading bugmail)]

i've fixed bookmarks to just crop right (grr), so making this bug more generic.

Comment 6

[Marcia Knous [:marcia]]

Netscape Nav Triage Team: changing comp to XP Toolkit

Comment 7

[Steve Dagley]

Anybody wanna comment if this is going to be fixed before the _next_ millenium?

Comment 8

[Marcia Knous [:marcia]]

nominating for dogfood (from sdagley's list of bugs that are good candidates for 
our next release) 

Comment 9

[Blake Ross]

cc'ing the usual suspect.

Comment 10

[:deanis74]

How do I change bookmarks back to crop in the center to replicate this bug?

Comment 11

[:deanis74]

OK, I see this now.  Shouldn't be that hard to fix.  Pink, give this one to me 
if you want and I'll take care of it sometime next week.

Comment 12

[:deanis74]

Attachment: fix

Comment 13

[:deanis74]

I got to this a little earlier than I thought.  I couldn't understand what was 
going on in the original code, so I rewrote it.  I changed the crop in 
bookmarks.xul to be center and it works as I would it expect it to.

Comment 14

[Mike Pinkerton (not reading bugmail)]

ask and ye shall receive.

Comment 15

[:deanis74]

Hey, that was easy.  Now how about someone looking this over for an r=?

Comment 16

[Mike Pinkerton (not reading bugmail)]

this doesn't account at all for the width of the ellipsis. won't the resulting 
string spill over |aWidth| by |ellipsisWidth|?

Comment 17

[:deanis74]

Actually, no.  Previously this was subtracted twice.  I found that it's already 
subtracted near the top of the function.

http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/nsTextBoxFrame.cpp#3
76

Comment 18

[Mike Pinkerton (not reading bugmail)]

ok then. r=pink

Comment 19

[:deanis74]

I moved the milestone to 0.9.1 last night, but that was probably too late.  
Moving to 0.9.2, although we can still get this into 0.9.1 if I get an sr=.  
I've already emailed Hyatt, so I'll fire something off to [e-mail].

Comment 20

[:deanis74]

I'll have to redo this now that bidi's in.

Comment 21

[:deanis74]

Attachment: new patch, with bidi

Comment 22

[:deanis74]

cc: ftang and mkaply to check out the bidi code.  It's a copy-and-paste from 
what was there already.

Comment 23

[timeless]

indentation is inconsistent 2v.4 spaces

+                if (totalWidth > aWidth)
+                    // greater than the allowable width
                     break;
+                else

else after break is like else after return. kill the else.

Comment 24

[:deanis74]

Don't know what happened with the indentation - I'll fix that and the 
"else" tonight.

Comment 25

[:deanis74]

I remember now.  For the spacing, I was trying to maintain a consistent look 
with the existing code.

Comment 26

[:deanis74]

New patch coming up.  I kept the indent at four spaces to be consistent with the 
rest of the file, and removed the "else"s after the "break"s.

Comment 27

[:deanis74]

Attachment: cvs diff -u

Comment 28

[:deanis74]

pink or timeless, can you give an r= on my new patch with the bidi code?  The 
bidi code hasn't changed at all, and that's the only change from the original 
patch that had r=pink.

Comment 29

[timeless]

r=timeless.

things to consider:
using nsLiteralString for ELLIPSIS (could be done now)
talking to arik about making ELLIPSIS localizable... (could be done shortly)
perhaps string iterators instead of integers (future)

Comment 30

[Mike Pinkerton (not reading bugmail)]

r=pink

Comment 31

[:deanis74]

timeless, I couldn't get nsLiteralString to compile, so let's add that to the 
"could be done shortly" bucket.

Blake, care to sr?

Comment 32

[Simon Montagu :smontagu]

r=[e-mail] for bidi

Comment 33

[Blake Ross]

We have to get this in for bug 85173.

What's the error trying to use nsLiteralString?

sr=blake

Comment 34

[:deanis74]

I couldn't get nsLiteralString, now apparently nsDependentString, to compile 
straight away.  I think my tree's pretty up-to-date, but I get:

'nsDependentString' : no appropriate default constructor available

I'm probably not understanding what timeless is asking for.  We're not hurting 
anything with how it is now, I don't think.  Since it's blocking bug 85173, I'm 
for checking this in, fixing up the bookmarks pane xul, and worrying about 
nsDependentString later (but soon, of course).

Comment 35

[Asa Dotzler [:asa]]

pushing out. 0.9.2 is done. (querying for this string will get you the list of
the 0.9.2 bugs I moved to 0.9.3)

Comment 36

[:deanis74]

There's an r=, an sr=, and I don't have cvs access.  Can someone check this in?

Comment 37

[Blake Ross]

Fix checked in.

Comment 38

[:deanis74]

Filed bug 90840: "Bookmarks should use crop=middle" to address the original 
intent of this bug.

Comment 39

[sairuh (rarely reading bugmail)]

mass-verifying claudius' Fixed bugs which haven't changed since 2001.12.31.

if you think this particular bug is not fixed, please make sure of the following
before reopening:

a. retest with a *recent* trunk build.
b. query bugzilla to see if there's an existing, open bug (new, reopened,
assigned) that covers your issue.
c. if this does need to be reopened, make sure there are specific steps to
reproduce (unless already provided and up-to-date).

thanks!

[set your search string in mail to "AmbassadorKoshNaranek" to filter out these
messages.]