919164 - Rename TEXT_FORCE_TRIM_WHITESPACE to TEXT_IS_IN_TOKEN_MATHML

Status: RESOLVED FIXED

(Core :: MathML, defect)

Comment 1

[James Kitchener (:jkitch)]

Attachment: rename

A simple find/replace in preparation for the changes in bug 114365 and bug 415413.  

There is also a minor comment typo in nsTextFrame.cpp that has been fixed.

Comment 2

[Frédéric Wang (:fredw)]

Comment on attachment 808271 [details] [diff] [review]
rename

Review of attachment 808271 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, that looks good to me. The state bits are currently not correctly set, though. I'm not sure that was tested after Eshan's fix for whitespace trimming. Could you try to see if setting the state bit correctly makes the following reftest work:

<table border="1">
  <tr>
    <td>
      <mtext>x</mtext>
    </td>
  </tr>
</table>

<table border="1">
  <tr>
    <td>
      <mtext>   x   </mtext>
    </td>
  </tr>
</table>

::: layout/mathml/nsMathMLTokenFrame.cpp
@@ +91,1 @@
>      }

The MathML token element has one single nsBlockFrame child and nsTextFrame grandchildren. Currently the code incorrectly sets the state bit on the nsBlockFrame child instead of the nsTextFrame grandchildren. Could you also fix that since it is needed for the two blocked bugs?

Comment 3

[Frédéric Wang (:fredw)]

Forget the reftest for now. I have tests in bug 415413 that shows that the flag is not set correctly, but I'm not sure I can get one that does not involve bug 415413.

Comment 4

[James Kitchener (:jkitch)]

Attachment: rename

David:  Your half of the patch (the first two files included) is a simple renaming and a minor typo correction.  The state bit is only used by MathML frames and will take on additional meanings in future bugs.

Fred: The additional code to select the correct frames has been imported from the experimental patch.  No reftest has been included per comment 3, but the two cases looked indistinguishable to me.

Comment 5

[Frédéric Wang (:fredw)]

Comment on attachment 808504 [details] [diff] [review]
rename

Review of attachment 808504 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me.

Comment 6

[Frédéric Wang (:fredw)]

Comment on attachment 808504 [details] [diff] [review]
rename

Asking review to roc for the renaming part, in case David is busy.

Comment 7

[Frédéric Wang (:fredw)]

No new tests since it's essentially some renaming and a trivial fix to nsMathMLTokenFrame::ForceTrimChildTextFrames.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2933d4d279c5

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2933d4d279c5