Created attachment 89746 [details]
test application

using the test application with input string :
"<i>Hello</i><span font_desc="20"><i>world</i></span>"

we get :

range: 0-5
desc = Italic

range: 5-10
desc = Normal 20

range: 10-2147483647
desc = Normal

the word "World" should be in italic

If I add a character (for example X) between <i> and <span we get a correct result:
"<i>Hello</i>X<span font_desc="20"><i>world</i></span>"

range: 0-5
desc = Italic

range: 5-6
desc = Normal

range: 6-11
desc = Italic 20

range: 11-2147483647
desc = Normal

Tested with the following version of Pango
- Pango 1.12.3 with glib 2.10.3 (with linux, Ubuntu dapper 6.06)
- Pango 1.14.5 with glib 2.12.4 (under win32)

This happens because markup parser uses pango_attr_list_change() to add parsed attributes and so causes the two <i> tags to be merged.

I'm simply changing it to a pango_attr_list_insert().  I can't imagine any serious undesired effect.  Owen?

2007-06-11  Behdad Esfahbod  <behdad@gnome.org>

        Bug 446355 – the parsing with pango_parse_markup is not coherent

        * pango/pango-markup.c (pango_parse_markup): Use
        pango_attr_list_insert() instead of pango_attr_list_change() as
        merging adjacent attributes of the same kind is not a safe operation
        and can change the derived font of a segment in an unexpected and
        incorrect way.

A) You are basically saying "pango_attr_list_change() doesn't work,
   let's not use it" => FIXED :-). Obviously delving into the innards
   of pango_attr_list_change() is never an appealing prospect, but
   I don't think we should just say that it doesn't work any more
   since we added font merging.

B) I can't *offhand* think of any downsides to using insert() in this
   case other than we'll produce inefficient attribute lists. Note that
   pango-markup.c is not trying at all for efficient attribute lists
   since it assumes that change() will do that for it.

(it's hard for me to guess whether there is much of an overall
win from producing more efficient lists for markup ... obviously there
will less memory usage and PangoAttrIterator will do less work, but 
you do more work to produce the lists...)


   

(In reply to comment #6)
> A) You are basically saying "pango_attr_list_change() doesn't work,
>    let's not use it" => FIXED :-). Obviously delving into the innards
>    of pango_attr_list_change() is never an appealing prospect, but
>    I don't think we should just say that it doesn't work any more
>    since we added font merging.

My line of thought was that _change() is not needed here because we are appending attributes at the end of the list mostly, and one attribute per one markup tag.  But I see your point about fixing _change().  The problem is that pango_attr_list_change() is documented in doing the merge.  So, changing its behavior is a slight API change.  The semantic change here of course is to consider all attributes affecting the font of the *same type* for merging  purposes.  Do you think that's fine?


> B) I can't *offhand* think of any downsides to using insert() in this
>    case other than we'll produce inefficient attribute lists. Note that
>    pango-markup.c is not trying at all for efficient attribute lists
>    since it assumes that change() will do that for it.

From what I understand, it generates exactly one attribute per markup tag.  So, I don't see any reason to try to merge them.

Bug 170055 may be relevant.

OK, I didn't look at the markup code close enough; it does look like the
resulting attributes should be fairly efficient in most cases even with
insert.


The docs for pango_attr_list_change() have an implied caveat that
the merging and replacing is not supposed to change the end effect of the 
resulting list over what insert() would produce. That's not even close
to right for fonts these days. There are various approaches that 
could be taken:

1) Make change() do the font description merging so that there is
   only ever one font attribute for any position the text.

2) Make change() sensitive to when identical (*) font description 
   attributes can be merged and when one font description attribute
   can replace another without changing meaning.

3) Just never do any merging of font description attributes at
   all.

3) is easiest to implement but breaks the basic idea of change() -
that you can apply an identical attribute in a loop and not have
the list grow without bound. 2) breaks the current logic of change()
and would require some significant rewriting. 1) is, I think, 
quite a bit easier, since the basic idea of the logic in change()
"one attribute of each type on each segment" is preserved. 

1) does, however, come close to an API change, though I'd consider
it within reason. It might be good to get Morten's feedback on whether
it would cause problems for gnumeric ... I know gnumeric was using
attribute lists in somewhat advanced ways, though I don't know if
they are using change() or not.

(*) One obvious breakage is that pango_font_description_equal() 
doesn't look at the mask at all, so it is unsuitable for use in pango_attr_font_desc_equal(); not the principal problem here, but
needs to be fixed.

Hmm, 1) does produce differences that are quite different from insert(),
though not that different from the current (broken) behavior of change().

   ------------------  italic
          -----------  bold
       --------------  style=normal, weight=normal

If those three are inserted with insert() in that order, then the
resulting list will have the three runs of different fonts:
    
   IIIINNNBBBBBBBBBBB

But if we make change() merge font descriptions, then the effect is 
different: you get:

   IIIINNNNNNNNNNNNNN

Since the information that the bold is a "later" attribute than
italic is lost. Now, actually, the proposed behavior is more in
line to what someone might expect than trying to copy the effect of
insert(), so I think it's OK.

Before digesting all your comments, lemme say that what makes it a lot more complicated code-wise is that we have lots of attributes playing here, not only font desc.  There are separate attributes for style, size, weight, stretch, etc...

2007-06-12  Behdad Esfahbod  <behdad@gnome.org>

        * pango/pango-attributes.c (pango_attr_font_desc_equal):
        Compare set fields of two font descriptions for equality
        too, as pango_font_description_equal() doesn't check the
        mask.

For markup I actually prefer inserting.  That better reflects what the user asked for in eg. "<i>...<span font_desc="Normal">...<i>...</i>...</span>...</i>".  With _insert, you get exactly three attributes that reflect the tags.

As for option 1) generating different results, I agree that the overriding is what the user expects.  I'll see if I can hack that.

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/pango/issues/80.