If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

VERIFIED FIXED in mozilla0.9.3

Status

()

Core
XUL
P3
normal
VERIFIED FIXED
17 years ago
15 years ago

People

(Reporter: Mike Pinkerton (not reading bugmail), Assigned: Dean Tessman)

Tracking

Trunk
mozilla0.9.3
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [br])

Attachments

(3 attachments)

(Reporter)

Description

17 years ago
- 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.
(Reporter)

Comment 1

17 years ago
makes bookmarks kinda hard to use. nsbeta3
Keywords: nsbeta3

Comment 2

17 years ago
nsbeta3-/future, need to make the column wider til beta5 (aka 6.0.1)
Whiteboard: [nsbeta3-]
Target Milestone: --- → Future

Comment 3

17 years ago
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.
Whiteboard: [nsbeta3-]

Comment 4

17 years ago
nsbeta3-
Whiteboard: [nsbeta3-]
(Reporter)

Comment 5

17 years ago
i've fixed bookmarks to just crop right (grr), so making this bug more generic.
Summary: URLs are start/end truncated, not middle truncated → nsTextBoxFrame's crop=middle is inverse of what is desired.

Updated

17 years ago
Status: NEW → ASSIGNED
Netscape Nav Triage Team: changing comp to XP Toolkit
Component: Bookmarks → XP Toolkit/Widgets
OS: All

Comment 7

17 years ago
Anybody wanna comment if this is going to be fixed before the _next_ millenium?
nominating for dogfood (from sdagley's list of bugs that are good candidates for 
our next release) 
Keywords: nsdogfood
Keywords: nsCatFood
Keywords: nsdogfood

Comment 9

17 years ago
cc'ing the usual suspect.
Keywords: nsbeta3
Hardware: Macintosh → All
Whiteboard: [nsbeta3-] → [br]
(Assignee)

Comment 10

17 years ago
How do I change bookmarks back to crop in the center to replicate this bug?
(Assignee)

Comment 11

17 years ago
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.
(Assignee)

Comment 12

17 years ago
Created attachment 33360 [details] [diff] [review]
fix
(Assignee)

Comment 13

17 years ago
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.
(Reporter)

Comment 14

17 years ago
ask and ye shall receive.
Assignee: evaughan → dean_tessman
Status: ASSIGNED → NEW
(Assignee)

Comment 15

17 years ago
Hey, that was easy.  Now how about someone looking this over for an r=?
Keywords: patch, review
(Reporter)

Comment 16

17 years ago
this doesn't account at all for the width of the ellipsis. won't the resulting 
string spill over |aWidth| by |ellipsisWidth|?
(Assignee)

Comment 17

17 years ago
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
(Reporter)

Comment 18

17 years ago
ok then. r=pink
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Target Milestone: Future → mozilla0.9.1
(Assignee)

Comment 19

17 years ago
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 reviewers@mozilla.org.
Target Milestone: mozilla0.9.1 → mozilla0.9.2
(Assignee)

Comment 20

17 years ago
I'll have to redo this now that bidi's in.
(Assignee)

Comment 21

17 years ago
Created attachment 34933 [details] [diff] [review]
new patch, with bidi
(Assignee)

Comment 22

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

Comment 23

17 years ago
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.
(Assignee)

Comment 24

17 years ago
Don't know what happened with the indentation - I'll fix that and the 
"else" tonight.
(Assignee)

Comment 25

17 years ago
I remember now.  For the spacing, I was trying to maintain a consistent look 
with the existing code.
(Assignee)

Comment 26

17 years ago
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.
(Assignee)

Comment 27

17 years ago
Created attachment 35091 [details] [diff] [review]
cvs diff -u
(Assignee)

Comment 28

17 years ago
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

17 years ago
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)
Keywords: review → approval
(Reporter)

Comment 30

17 years ago
r=pink
(Assignee)

Comment 31

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

Blake, care to sr?
r=simon@softel.co.il for bidi

Comment 33

17 years ago
We have to get this in for bug 85173.

What's the error trying to use nsLiteralString?

sr=blake
Blocks: 85173
(Assignee)

Comment 34

17 years ago
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

17 years ago
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)
Target Milestone: mozilla0.9.2 → mozilla0.9.3
(Assignee)

Comment 36

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

Comment 37

16 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Assignee)

Comment 38

16 years ago
Filed bug 90840: "Bookmarks should use crop=middle" to address the original 
intent of this bug.
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.]
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.