After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 735732 - Skip attributes inside elided text
Skip attributes inside elided text
Status: RESOLVED FIXED
Product: pango
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: pango-maint
pango-maint
Depends on:
Blocks: 668258
 
 
Reported: 2014-08-30 23:31 UTC by Matthias Clasen
Modified: 2014-09-03 19:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Skip attributes inside elided text (1.32 KB, patch)
2014-08-30 23:31 UTC, Matthias Clasen
needs-work Details | Review
PangoGlyphItem: Better treatment of ellipsized runs (2.32 KB, patch)
2014-09-02 22:20 UTC, Matthias Clasen
committed Details | Review

Description Matthias Clasen 2014-08-30 23:31:13 UTC
When we insert a run for the ellipsis in the layout, we are careful
to preserve attributes of the elided text. This means that the run
can have attributes which do not cover its full range, unlike
'normal' runs whose attributes always span the entire range.

PangoRenderer was not paying attention to the ranges and just
blindly applied all attributes it found. The effect of this is
that the ellipsis in a label with links is blue and underlined
even if the link is entirely contained in the elided text.

Correct this ignoring applying attributes that begin after the
start of the run.
Comment 1 Matthias Clasen 2014-08-30 23:31:16 UTC
Created attachment 284912 [details] [review]
Skip attributes inside elided text
Comment 2 Matthias Clasen 2014-09-02 17:36:15 UTC
Review of attachment 284912 [details] [review]:

I made Owen look at this for - the upshot is that pango_glyph_item_apply_attrs needs to be smarter instead, and avoid applying no-shape attributes whose range is wholly contained in the ellipsis run.
Comment 3 Matthias Clasen 2014-09-02 22:20:33 UTC
Created attachment 285210 [details] [review]
PangoGlyphItem: Better treatment of ellipsized runs

When we reapply non-shape attributes, we must take care to
not add them inside ellipsized runs, or we end up with a blue,
underlined ellipsis if there is a link anywhere inside the
ellipsized text.
Comment 4 Behdad Esfahbod 2014-09-03 08:37:11 UTC
Note that even for normal runs the attributes do not necessarily expand the entire run.  Underline is a good example, so is color.
Comment 5 Behdad Esfahbod 2014-09-03 08:49:45 UTC
Didn't review the patch too closely, but looks about right.
Comment 6 Matthias Clasen 2014-09-03 12:14:23 UTC
(In reply to comment #4)
> Note that even for normal runs the attributes do not necessarily expand the
> entire run.  Underline is a good example, so is color.

If thats the case, I don't understand how pango_renderer_default_prepare_run can get away with not looking at the ranges.
Comment 7 Owen Taylor 2014-09-03 13:53:00 UTC
(In reply to comment #4)
> Note that even for normal runs the attributes do not necessarily expand the
> entire run.  Underline is a good example, so is color.

This is not the case - the whole idea of the code that this patch modifies is to split runs up after shaping to take attributes like color and underline into account. (As a consequence of the fact that attributes always apply to a run, an attribute can never apply to just one character in a cluster - try pango-view --font 32 --markup -t '<u>f</u>ile')
Comment 8 Behdad Esfahbod 2014-09-03 14:07:42 UTC
Ah, sorry, my bad.  I should have said ideally attributes wouldn't apply to entire runs after we fix color/underlining parts of ligatures.
Comment 9 Owen Taylor 2014-09-03 14:19:19 UTC
Review of attachment 285210 [details] [review]:

::: pango/pango-glyph-item.c
@@ +547,3 @@
+                   gint            range_end)
+{
+  /* Skip attributes that are wholly contained in an ellipsized run */

The rule that ellipsize.c uses is that the attributes applying to the ellipsis are the attributes applying to the *first character* in the ellipsized text. For me:

pango-view  --font 'Adwaita 32' --markup -t '<small>...small</small><big>Big</big><small>small...</small>' --ellipsize=middle --width 240

Shows this - the ellipsis is big while everything else is small. If we apply this rule here as well, we won't entirely fix the original problem, of course, since if the ellipsis starts *exactly* where the link starts, the ellipsis will be shown as a link. But it might be good enough - it's not *entirely* weird if:

"Please go to our website _www.gnome.org_ for further details" gets ellipsized to "Please go to our website _..._"

Better than the current situation where you can have:

"Please _..._"

and much less likely to be triggered. I *don't* think we want inconsistent rules, so we'd have to adjust ellipsize.c as well if we wanted to prevent this problem.

If we're applying the existing rule, I think we just need to add "is_a_ellipsis || .." to:

  /* Short circuit the case when we don't actually need to
   * split the item
   */
  if (range_start <= glyph_item->item->offset &&
      range_end >= glyph_item->item->offset + glyph_item->item->length)
    goto out;

We've found the range that includes the first character in the run, at that point, we should just copy it's attributes over to the item without splitting it.
Comment 10 Owen Taylor 2014-09-03 14:19:21 UTC
Review of attachment 285210 [details] [review]:

::: pango/pango-glyph-item.c
@@ +547,3 @@
+                   gint            range_end)
+{
+  /* Skip attributes that are wholly contained in an ellipsized run */

The rule that ellipsize.c uses is that the attributes applying to the ellipsis are the attributes applying to the *first character* in the ellipsized text. For me:

pango-view  --font 'Adwaita 32' --markup -t '<small>...small</small><big>Big</big><small>small...</small>' --ellipsize=middle --width 240

Shows this - the ellipsis is big while everything else is small. If we apply this rule here as well, we won't entirely fix the original problem, of course, since if the ellipsis starts *exactly* where the link starts, the ellipsis will be shown as a link. But it might be good enough - it's not *entirely* weird if:

"Please go to our website _www.gnome.org_ for further details" gets ellipsized to "Please go to our website _..._"

Better than the current situation where you can have:

"Please _..._"

and much less likely to be triggered. I *don't* think we want inconsistent rules, so we'd have to adjust ellipsize.c as well if we wanted to prevent this problem.

If we're applying the existing rule, I think we just need to add "is_a_ellipsis || .." to:

  /* Short circuit the case when we don't actually need to
   * split the item
   */
  if (range_start <= glyph_item->item->offset &&
      range_end >= glyph_item->item->offset + glyph_item->item->length)
    goto out;

We've found the range that includes the first character in the run, at that point, we should just copy it's attributes over to the item without splitting it.
Comment 11 Matthias Clasen 2014-09-03 19:23:45 UTC
Attachment 285210 [details] pushed as d23a0b0 - PangoGlyphItem: Better treatment of ellipsized runs