title is not shortened properly when containing multi-byte characters

RESOLVED DUPLICATE of bug 898984

Status

()

Core
XUL
RESOLVED DUPLICATE of bug 898984
3 years ago
2 years ago

People

(Reporter: sam113101, Unassigned, Mentored)

Tracking

37 Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Comment hidden (obsolete)
(Reporter)

Comment 1

3 years ago
Sorry, I tried to paste the original link (which contained weird characters) and it seems to have messed with firefox's post data, or maybe it's a bug or limitation in the bugzilla software.

Here's the entire post:

Go there: http://www.reddit.com/r/Random_Acts_Of_Pizza/comments/330cr6/
Resize the window so that the title has to be shortened.

The title is shortened improperly. First, the text does not reach the right end of the tab as it's supposed to. Second, it seems to me that firefox is trying to split a multi-byte character in half (which shows up as a rectangle with an hexadecimal number in it).

The text should reach the right end of the tab and there should be no weird rectangles. My hypothesis is that firefox is counting the number of *bytes* rather than the number of *unicode characters*, which gives the wrong result. I could be wrong.
I can reproduce this on Nightly.
Status: UNCONFIRMED → NEW
Component: Untriaged → Tabbed Browser
Ever confirmed: true

Comment 3

3 years ago
Boris, I recall that for bug 1104875 you had to fiddle to make this not happen. I presume we need similar fiddling in XUL layout so that the cropping works correctly instead of mashing the multibyte things? Don't know if it's harder here because we are also trying to fit a particular width...
Component: Tabbed Browser → XUL
Flags: needinfo?(bzbarsky)
Product: Firefox → Core

Comment 4

3 years ago
(also, I'm surprised this isn't a dupe but I can't find any...)

Comment 5

3 years ago
Looks like we need to handle the width differently in nsTextBoxFrame::CalculateTitleForWidth, which has a note to this effect just before the switch statement.
Yes, comment 5 is spot on.  Instead of passing single chars to AppUnitWidthOfString we should be checking for surrogates, then passing the entire surrogate pair in as needed.  As in, use the (char16_t*, length) overload of AppUnitWidthOfString.
Flags: needinfo?(bzbarsky)

Updated

3 years ago
Flags: needinfo?(gijskruitbosch+bugs)

Comment 7

3 years ago
Created attachment 8608121 [details] [diff] [review]
Something like the following

Will work on some tests.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
You probably need to check that the next code unit is the other half of the surrogate pair too.  consider CropRight when you have a high surrogate followed by a not-low-surrogate code unit.  In that case you don't really want to measure that not-low-surrogate code init together with the high surrogate.

Updated

3 years ago
Flags: needinfo?(gijskruitbosch+bugs)

Updated

3 years ago
Duplicate of this bug: 1125861

Updated

3 years ago
Assignee: enndeakin → nobody
Mentor: enndeakin
Status: ASSIGNED → NEW

Comment 10

3 years ago
Created attachment 8668464 [details] [diff] [review]
Updated patch

This was the latest patch I had written. It seems to work, but the test does not since it relies on there being some font with a mostly predictable size with the right characters in it being available on all platforms. I'm sure that can fixed somehow but I moved on to other things before investigating that.
Attachment #8608121 - Attachment is obsolete: true
It might be worth adding some non-BMP characters to Ahem?

Comment 12

2 years ago
Just in case: bug 898984 describes the same issue
See Also: → bug 898984

Updated

2 years ago
Duplicate of this bug: 1277894

Updated

2 years ago
Duplicate of this bug: 1294441
It seems to me this is indeed a duplicate of bug 898984.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 898984
See Also: bug 898984
You need to log in before you can comment on or make changes to this bug.