407861 - Bolding the found text in autocomplete breaks ligatures

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Simon Montagu :smontagu]

Attachment: screenshot -- searching for Arabic Qaf

In the screenshot the highlighted letter Qaf should connect to the surrounding letters: the first entry should appear as جريدة القاهرة, not جريدة ال‌ق‌اهرة.

The new text run code would handle this correctly if the highlighted text were styled with a color change instead of font-weight.

Comment 1

[(not reading, please use [e-mail] instead)]

Simon, if we switched from <label> to <html:span>, would this be fixed?

I think we're going to switch to html:span anyways, for the RTL issues.

Comment 2

[Simon Montagu :smontagu]

Changing to html:span won't fix this without the change from font-weight to color. I don't know if changing to color will fix it without changing to html:span, but I suspect not.

Comment 3

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

I spoke with Pav about this - using just underline will work here. Italic will get us in similar problems as bold.

Comment 4

[(not reading, please use [e-mail] instead)]

> I spoke with Pav about this - using just underline will work here. Italic will get us in similar problems as > bold.

Mike, are you suggesting that we should stop bolding and just do underline?

Note, sayre suggested in bug #406254 that instead of bold we could use text color instead of bold for emphasis.

Comment 5

[(not reading, please use [e-mail] instead)]

actually, I'm not 100% sure that we can't still do bold and html:span.

using ted's live xul editor (ted, you rule!), the problem, once again, is using label (which breaks ligatures) as opposed to html:span.

I'll attach some xul that should show that this will work, but I need Simon's expert eye to confirm.

Comment 6

[(not reading, please use [e-mail] instead)]

Attachment: test case showing that html:span works, but label does not

Comment 7

[(not reading, please use [e-mail] instead)]

actually, upon zooming in to get a better look, pav is right, bold breaks the ligature even if we use html:span.

Comment 8

[(not reading, please use [e-mail] instead)]

Attachment: better test case, showing bold fails (with both label and span), but that underline works with span

Comment 9

[Alex Faaborg [:faaborg] (Firefox UX)]

Are there any languages where breaking ligatures actually changes the meaning of the text, or does it only change the aesthetically pleasing kerning?  Quickly scanning the wikipedia article, this appears to be the case: "The Arabic alphabet, historically a cursive derived from the Nabataean alphabet, most letters take a variant shape depending on which they are followed (word-initial), preceded (word-final) or both (medial) by other letters." http://en.wikipedia.org/wiki/Typographical_ligature#Non-Latin_alphabets

Also, is there anyway for us to fall back to just underlining only in situations where we are displaying characters from a language that requires ligatures? (similar to how my carat in this text field is picking up a tick mark on the right?)

Comment 10

[Dão Gottwald [:dao]]

Attachment: proposal

This is based on attachment 294887 [details] and wouldn't break ligatures.

Comment 11

[Tony Mechelynck [:tonymec]]

In reply to comment #9
Yes, in Arabic many letters take up a "tail" or "flourish" when they are word-final (i.e. in the final or isolate presentation form). Replacing two joined letters of the same word by a word-final + word initial shape would mean introducing a word break, which may or may not give a "meaningful" result, but certainly not the "intended" meaning.

In addition, in Arabic the lettergroup lam + alif is mandatorily represented as a single glyph. Using (initial or medial lam) + (final alif) would be more than untediousness, it would be a fault. I'm not convinced that it would be possible to mark the boundary "in the middle of" such a glyph in any way -- now the Arabic article "al" is written as a single word with what follows it, so that searching for a word starting in alif would, if it were preceded by the article, require exactly such an "impossible" boundary.

Comment 12

[Alfred Kayser]

Attachment: Even better testcase: really showing differences between span, label and multiple descriptions

The previous testcase didn't remove the margin/padding of the label, so the comparision was not really fair. Also added a hbox of <description> to see if ligatures survive that. (they should...)

Comment 13

[Simon Montagu :smontagu]

I think the testcases show that underlining instead of bolding isn't a great solution for Arabic, because many glyphs go down below the baseline and the underline tends to make the text significantly less readable :(

Comment 14

[Dão Gottwald [:dao]]

(In reply to comment #13)
> underline tends to make the text significantly less readable :(

We highlight what you've just typed, so this shouldn't be a big deal. The *context* of the text is interesting.

Comment 15

[Simon Montagu :smontagu]

Attachment: Testcase with html:span only, using bold, underline and color

<html:span> is a done deal (bug 402118) so IMO the only question is between underline and color. (text-decoration: blink also works well)

Comment 16

[Mike Connor [:mconnor]]

All of the above suck for various reasons:

Bolding breaks ligatures.
Underlining hurts readability in some locales
Text color, other than selection-like highlight, is not sec508 compliant, since high-contrast users, etc, won't be able to distinguish much.

Comment 17

[Dão Gottwald [:dao]]

Attachment: updated proposal

With highlight colors and without underline, as already proposed a week ago via e-mail.

Comment 18

[Ed Lee :Mardak]

(In reply to comment #14)
> (In reply to comment #13)
> > underline tends to make the text significantly less readable :(
> We highlight what you've just typed, so this shouldn't be a big deal. The
> *context* of the text is interesting.
With bug 415403, we would show different terms being highlighted, so it could be useful to see what is being matched where.

So font-color and background-color should work perhaps with a border with the same color as the highlight and negative margin so that the highlighted area is just a little bit bigger than the text it selects.

Comment 19

[Jesse Ruderman]

The attachment in comment 17 is ugly.  It's hard to read text when it jumps from dark-on-light to light-on-dark.  A yellow highlight would be better.

Comment 20

[Dão Gottwald [:dao]]

I've used this for several weeks without realizing that this is hard to read. That's because it's actually not hard to read and/or because I spot but don't read the highlighted text, since I just typed it.

(In reply to comment #18)
> So font-color and background-color should work perhaps with a border with the
> same color as the highlight and negative margin so that the highlighted area is
> just a little bit bigger than the text it selects.

Border would break ligatures, outline with or without an offset would overlap the highlighted or the adjacent text.

Comment 21

[Jesse Ruderman]

I think we should be open to the possibility of using a different indicator depending on language.  (For example, Google uses bold for English but red for Chinese.)  Maybe the best thing to do is bold+underline for English but underline only for Arabic.

Comment 22

[Simon Bünzli]

(In reply to comment #19)
> It's hard to read text when it jumps from dark-on-light to light-on-dark.

Except that's what you get when you select text in a text box or even any text on this page, when you cursor down any menu or tree (e.g. in the bookmarks sidebar) or when using autoFill - IOW: whenever something is highlighted. So: why do something different only in _this_ case?

Comment 23

[Jesse Ruderman]

Because this isn't a selection, and the immediate context is more important.

Comment 24

[Mike Connor [:mconnor]]

Need to make a decision on this today.

Comment 25

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

(In reply to comment #21)
> I think we should be open to the possibility of using a different indicator
> depending on language.  (For example, Google uses bold for English but red for
> Chinese.)  Maybe the best thing to do is bold+underline for English but
> underline only for Arabic.

Do we have an easy way to do this?

Comment 26

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

No. Plus, what would you do if the search term is half Arabic and half English?

Comment 27

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

I thought the idea was that when parsing the results to determine what section matches against the result (ie: what section to apply the .ac-emphasize-text rule to) we could also determine if the character set was one that required something like .ac-emphasize-text-intl.

I also notice that we have an intl.css; is this something we can just leave to localizers?

Comment 28

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

I guess JS could look for Arabic characters (or characters that need similar treatment) in the search terms and mark matches with something like .ac-emphasize-text-alt. That would use a highlight like in https://bugzilla.mozilla.org/attachment.cgi?id=304080, which is ugly but probably our only option (can't use underline, can't use bolding or any other font change, can't use text color). Otherwise we'd use bold+underline like today.

Comment 29

[Ryan A. C.]

Attachment: Using Highlight color as text color

Note that while I used generic Blue for the mockup, this color should be the system highlight color

Comment 30

[Dão Gottwald [:dao]]

highlight is a background color, not a text color. The text color would be highlightText, and it has to be used together with the highlight background. Otherwise you'll get text that's either invisible or not differentiable from normal text with random OS themes.

Comment 31

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

Comment on attachment 304080 [details]
updated proposal

icky :(

Comment 32

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

I'm still waiting for someone to tell me if we can handle this in intl.css - IMO it's not worth making 99% of the locales look worse because we break things in 1% of the locales.

Also, it's unclear to me if what we're doing is making the text look worse, or actually totally breaking the rendering of the text.

Comment 33

[Simon Montagu :smontagu]

(In reply to comment #32)
 
> I'm still waiting for someone to tell me if we can handle this in intl.css -
> IMO it's not worth making 99% of the locales look worse because we break things
> in 1% of the locales.
>

For the record 7.5% of the locales released in Beta4 have this problem (Arabic, Gujarati and Punjabi), but anyway locales and intl.css are pretty much orthogonal to this -- you don't need to be in an Arabic locale to surf pages with Arabic titles.

> Also, it's unclear to me if what we're doing is making the text look worse, or
> actually totally breaking the rendering of the text.
> 

Breaking it. See comment 11.

Comment 34

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

Is there a good list of what character sets this breaks? I've got Mardak looking into the solution posited in comment 28, where if we detect any such characters in the search string we use .ac-emphasize-text-alt which can fall back to something like underline-only.

Comment 35

[Ed Lee :Mardak]

Attachment: screenshot of v1

Using 2 types of emphasis.

Comment 36

[Ed Lee :Mardak]

Attachment: v1

Detect when certain emphasis text areas need to be alternately emphasized. It's possible for a title/url/tag to have different combinations of emphasis because only the part that needs alternate emphasis gets the alternate.

Comment 37

[Ed Lee :Mardak]

Attachment: screenshot of v1 w/ tags

Comment 38

[Dão Gottwald [:dao]]

Comment on attachment 310266 [details] [diff] [review]
v1

>+  color: red;

That's going to be another possible vector for bug 409974.

Comment 39

[Ed Lee :Mardak]

I know. ;) I was going to do something crazy like color: red; background-color: green; just to make sure people knew it wasn't the final color.

Comment 40

[Tony Mechelynck [:tonymec]]

In reply to comment #35 and comment #37
Mardak: What happens when the highlight boundary is halfway a ligature? E.g., searching for اسلام "Islam" and expecting to find also الاسلام "al-Islam" (meaning: the Islam)

Comment 41

[Ed Lee :Mardak]

I didn't quite understand.. but I think you're asking something similar to

"What if you matched "<ligature><non-ligature>" (no spaces) when searching for "<ligature> <non-ligature>?" Both will be emphasized as non-ligature, e.g., all red.

Comment 42

[Ed Lee :Mardak]

FYI, it's not really matching <ligature> vs <non-ligature>. Right now it's just checking if the unicode value is in the arabic ranges. So it'll be alternately styled even if it didn't have ligatures.

What other ranges should we include for the alternate?

Comment 43

[Tony Mechelynck [:tonymec]]

(In reply to comment #41)
> I didn't quite understand.. but I think you're asking something similar to
> 
> "What if you matched "<ligature><non-ligature>" (no spaces) when searching for
> "<ligature> <non-ligature>?" Both will be emphasized as non-ligature, e.g., all
> red.
> 

What I mean is, trying to match the alif part but not the laam part of a laam-alif digraph, as with the example I gave: when searching for اسلام (Islam) I expect to find also الاسلام (meaning "the Islam"). How will that be highlighted, considering that the match starts halfway the لا laam-alif digraph? Could you give a screenshot?

Comment 44

[Ed Lee :Mardak]

Is that related to this bug? What do we do right now on trunk? Unless "the" of "the Islam" is before/after "Islam", I don't think it would match.

Pretend it was english. Would "Islam" match "Isthelam"? (But I have no idea how the language works..)

Comment 45

[Ed Lee :Mardak]

Attachment: screenshot of v1 (the islam)

That's interesting... ;)

Comment 46

[Ed Lee :Mardak]

Attachment: screenshot of v1 "t h e" of (the islam)

Oh. It makes more sense now that I realize "the" is made up of two characters ا ل which happens to be the same starting character in islam. Seems like both the letter pairs ال and لا appear in "the islam" except the latter has a special ligature.

Comment 47

[Tony Mechelynck [:tonymec]]

Exactly. "The" of "the Islam" is before (right of, in this RTL language) "Islam", BUT:

1. The article "al" is always written together with its substantive (which it precedes), with no intervening space, as if they were one word;
2. Whenever the letter laam (including the ending -l of the article) is followed by the letter alif (including the first letter of any word beginning in a vowel) with no intervening space, both MUST be written as a digraph.

In the V-like لا sign near the beginning (right end) of الاسلام , the curved line on the right and bottom is the laam (l of the article al- "the"), and the oblique line on the left is the alif (i of "Islam"). When "Islam" is written اسلام without the article, that same initial alif of "Islam" is the vertical letter at far right; while in الاسلام (al-Islam) the initial vertical at far right of the "article+substantive" is the initial alif representing the a- of the "al" article.

And thanks for that screenshot. :-)

Comment 48

[Simon Montagu :smontagu]

(In reply to comment #42)
> What other ranges should we include for the alternate?

As an opening bid I would say just include U+0600 through U+109F (Arabic, Syriac and all the Indic languages) The Arabic Presentation Forms blocks at U+FB50 and U+FE70 actually don't need to be included, because they are precomposed ligatures and connecting forms.

Comment 49

[Ed Lee :Mardak]

Attachment: v1.1

(In reply to comment #48)
> As an opening bid I would say just include U+0600 through U+109F (Arabic,
> Syriac and all the Indic languages)
Sounds good. Makes it simpler to check as well. :)

Keeping it as red for now, but it'll eventually probably depend on the url color (per platform).

gavin: This is right after bug 407946 in my stack (and before bug 415403).

Comment 50

[Dão Gottwald [:dao]]

Well, initially we also kept the green "for now", knowing that it would cause trouble. Months later, it's still not fixed. Now that we're even closer to the release, I would definitely reject another hard coded color ... but of course I'm neither a peer nor doing ui reviews.

Comment 51

[Ed Lee :Mardak]

Attachment: v1.2

No additional color.. just underline now.

Comment 52

[Ed Lee :Mardak]

Attachment: screenshot of v1.2 "t h e" of (the islam)

Comment 53

[Ed Lee :Mardak]

You can try out this build (apparently windows and linux only):

https://build.mozilla.org/tryserver-builds/2008-03-19_11:[e-mail]/

It has the alternate emphasis as underline.

Bug 335418 - Pause downloads when computer is about to go to sleep
Bug 407946 - emphasize all matching text in title and url, not just the first match in title and url
** Bug 407861 - Bolding the found text in autocomplete breaks ligatures
Bug 415403 - Show matches for all search words for location bar autocomplete
Bug 418257 - Show what part of which tags match for urlbar autocomplete
Bug 392143 - show keywords as url bar autocomplete choices
Bug 249468 - Add all bookmark keywords to location bar autocomplete drop-down list
Bug 395161 - make it possible to restrict the url bar autocomplete results to bookmarks, tags, or history entries
Bug 407204 - adjust the title and url text sizes
Bug 406257 - reduce the number of rows in url bar autocomplete from 10 to 6
Bug 420437 - Search and emphasize quoted strings with spaces
Bug 414326 - Use DownloadUtils for software update downloads
Bug 422491 - Optimize AwesomeBar if it finished searching and found fewer than maxResults
Bug 422698 - different levels of URL decoding for address bar and autocomplete suggestions

Comment 54

[Ed Lee :Mardak]

Attachment: v1.3

Clean up the autocomplete/browser.css files so that autocomplete.css keeps the structural and browser.css gets the styling.

This build should correctly bold things on windows again:

https://build.mozilla.org/tryserver-builds/2008-03-19_22:[e-mail]/

Comment 55

[Dão Gottwald [:dao]]

(In reply to comment #54)
> Clean up the autocomplete/browser.css files so that autocomplete.css keeps the
> structural and browser.css gets the styling.

fwiw, that distinction doesn't really exist ... both are theme files. It might actually be better to keep the styling in toolkit, so that a non-browser autocomplete consumer get an emphasis without having to add custom CSS. Or is there a reason for not emphasizing that stuff by default?

Comment 56

[Ed Lee :Mardak]

Attachment: v1.4

Move emphasis styling back into toolkit and cleaned up selectors.

html|*.ac-emphasize-text -> .ac-normal-text > html|span

Comment 57

[Ed Lee :Mardak]

Attachment: screenshot of trunk vs v1.4

Comment 58

[Ed Lee :Mardak]

stephend: "Create a bookmark with title 'الاسلام (The Islam)' (copy/paste) and url 'http://theislam/الاسلام'. Search for 'ل' (copy/paste) in the location bar. Make sure the result *doesn't* look like the left result in attachment 310832 [details]. Notably, the emphasized part should *not* look like a 'J'. Instead it should look like the right side in terms of it not emphasizing the match as 'J'."

Comment 59

[Ed Lee :Mardak]

Comment on attachment 310831 [details] [diff] [review]
v1.4

a1.9b5? for P1 bug.

litmus test is described in comment #58.

Comment 60

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

Comment on attachment 310831 [details] [diff] [review]
v1.4

a=beltzner, yay!

Comment 61

[Ed Lee :Mardak]

Checking in toolkit/content/widgets/autocomplete.xml;
/cvsroot/mozilla/toolkit/content/widgets/autocomplete.xml,v  <--  autocomplete.xml
new revision: 1.126; previous revision: 1.125
done
Checking in toolkit/themes/gnomestripe/global/autocomplete.css;
/cvsroot/mozilla/toolkit/themes/gnomestripe/global/autocomplete.css,v  <--  autocomplete.css
new revision: 1.21; previous revision: 1.20
done
Checking in toolkit/themes/pinstripe/global/autocomplete.css;
/cvsroot/mozilla/toolkit/themes/pinstripe/global/autocomplete.css,v  <--  autocomplete.css
new revision: 1.21; previous revision: 1.20
done
Checking in toolkit/themes/winstripe/global/autocomplete.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/autocomplete.css,v  <--  autocomplete.css
new revision: 1.31; previous revision: 1.30
done

Comment 62

[Stephen Donner [:stephend] Not actively reading bugmail]

in-litmus+

https://litmus.mozilla.org/show_test.cgi?id=5212

Comment 63

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b5pre) Gecko/2008032204 Minefield/3.0b5pre

 Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b5pre) Gecko/2008032204 Minefield/3.0b5pre

-and-

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b5pre) Gecko/2008032204 Minefield/3.0b5pre