Closed Bug 957128 Opened 7 years ago Closed 7 years ago

Soft hyphen not visible in container styled with word-wrap:break-word


(Core :: Layout: Text and Fonts, defect)

Not set





(Reporter: filipnohe, Assigned: mats)




(4 files)

Attached file Test case
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 (Beta/Release)
Build ID: 20131205075310

Steps to reproduce:

A paragraph has the CSS property "word-wrap: break-word" set, or has the property "word-wrap: break-word" inherited from a parent element.

A word that is too long to fit the paragraph has a manual soft hyphen (­ or Unicode U+00AD) added.

Actual results:

The word is broken where the soft hyphen is, but the hyphen glyph is not displayed.

Expected results:

The word should be broken where the soft hyphen is, and the hyphen glyph should be displayed.
Workaround: use hyphens instead of word-wrap and ­
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86_64 → All
Version: 26 Branch → Trunk
Attached file Workaround
A more SEO compatible workaround is to put the another ­ somewhere before the one where the word should be split. There has to be at least one “normal” character between the two ­s.
(In reply to bugzilla from comment #2)
> Created attachment 8357099 [details]
> Workaround

Interesting, thanks!

That workaround illuminates the bug quite clearly...
which is that the assignment to 'lastBreakUsedHyphenation' is not using 'hyphenation',
whereas '*aBreakPriority' is.  After setting "*aBreakPriority" to eNormalBreak, the
remaining iterations in the loop will set 'wordWrapping' to false (line 6294) because
*aBreakPriority <= eWordWrapBreak is false, so any &shy; after the first will set
'lastBreakUsedHyphenation' to true.
Attached patch fix+reftestsSplinter Review
Assignee: nobody → matspal
Attachment #8358853 - Flags: review?(smontagu)
Attached patch optional part 2Splinter Review
This is an idempotent change to make the code slightly clearer.
I think it should be at least as fast as the previous version but
I haven't benchmarked it.  I'll be happy to do a microbenchmark
on request.
Attachment #8358854 - Flags: review?(smontagu)
Attachment #8358853 - Flags: review?(smontagu) → review+
Comment on attachment 8358854 [details] [diff] [review]
optional part 2

Review of attachment 8358854 [details] [diff] [review]:

Yes, this improves clarity a great deal. I think that this code is sufficiently on a performance-critical path that you need to do the micro-benchmark for due diligence, though.
Attachment #8358854 - Flags: review?(smontagu) → review+
Duplicate of this bug: 959556
I micro-benchmarked the if-block that I changed:
and that block became ~5% faster with the change, in Linux64 and OSX Opt builds.
Flags: in-testsuite+
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.