Closed Bug 957128 Opened 6 years ago Closed 6 years ago

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

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: filipnohe, Assigned: mats)

References

Details

Attachments

(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 ­
https://developer.mozilla.org/en-US/docs/Web/CSS/hyphens
Status: UNCONFIRMED → NEW
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...
http://hg.mozilla.org/mozilla-central/annotate/e89afc241513/gfx/thebes/gfxFont.cpp#l6309
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.

https://tbpl.mozilla.org/?tree=Try&rev=b4bf4585dd61
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:
https://hg.mozilla.org/integration/mozilla-inbound/file/0c8fa25eafe9/gfx/thebes/gfxFont.cpp#l6394
and that block became ~5% faster with the change, in Linux64 and OSX Opt builds.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3ef6bc080fa3
https://hg.mozilla.org/integration/mozilla-inbound/rev/0c8fa25eafe9
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/3ef6bc080fa3
https://hg.mozilla.org/mozilla-central/rev/0c8fa25eafe9
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.