355267 - Remove svg fragment tree code

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[tor]

It seems that a lot of the fragment tree code is redundant with glyph positioning, and could be removed.

Comment 1

[tor]

Attachment: fragment tree removal

Initial attempt - passed basic tests, but I don't have a dynamic text testsuite laying around.

Comment 2

[Robert Longson [:longsonr]]

Lovely. I always wondered what the point of that code was.

You need to update the nsISVGGlyphFragmentNode.h ID.

Also dynamic updates of xml:space are now broken. I'll attach a testcase, you click on some bits of it and the text should all compress. However what happens now is that it goes into

void
nsSVGTextFrame::UpdateGlyphPositioning()
{
  if (mMetricsState == suspended)
    mPositioningDirty = PR_TRUE;

  if (!mPositioningDirty)
    return;

  SetWhitespaceHandling();

The metrics state is not suspended so it returns as the positioning is not dirty without setting the new whitespace handling or refreshing the display.

Comment 3

[Robert Longson [:longsonr]]

Attachment: testcase

Comment 4

[tor]

Attachment: simple approach to fixing reported problem

Comment 5

[Robert Longson [:longsonr]]

(In reply to comment #4)
> Created an attachment (id=242542) [edit]
> simple approach to fixing reported problem
> 

This does work for my testcases. However if a mMetricsState is suspended and we get an xml:space attribute change the metrics state will change to unsuspended which I don't think we want.

Comment 6

[tor]

Attachment: tweak the logic again

Comment 7

[Robert Longson [:longsonr]]

(In reply to comment #6)
> Created an attachment (id=244122) [edit]
> tweak the logic again
> 

Great stuff. Did you mean to set a review request on this?

Comment 8

[tor]

Not quite yet - it's asserting under some conditions and I want to check that out first.

Comment 9

[tor]

Attachment: finished, I think

Comment 10

[Robert Longson [:longsonr]]

(In reply to comment #9)
> Created an attachment (id=245258) [edit]
> finished, I think
> 

You have included the files from bug 359449 in this patch.

Comment 11

[tor]

Attachment: just the fragment removal

Comment 12

[tor]

Checked in.