Rename TEXT_FORCE_TRIM_WHITESPACE to TEXT_IS_IN_TOKEN_MATHML

RESOLVED FIXED in mozilla27

Status

()

Core
MathML
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jkitch, Assigned: jkitch)

Tracking

unspecified
mozilla27
x86_64
Windows 8
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Updated

5 years ago
Assignee: nobody → jkitch.bug
Blocks: 114365, 415413
(Assignee)

Comment 1

5 years ago
Created attachment 808271 [details] [diff] [review]
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.
Attachment #808271 - Flags: review?(fred.wang)
Attachment #808271 - Flags: review?(dbaron)
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?
Attachment #808271 - Flags: review?(fred.wang) → feedback+
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.
(Assignee)

Updated

5 years ago
Attachment #808271 - Flags: review?(dbaron)
(Assignee)

Comment 4

5 years ago
Created attachment 808504 [details] [diff] [review]
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.
Attachment #808271 - Attachment is obsolete: true
Attachment #808504 - Flags: review?(fred.wang)
Attachment #808504 - Flags: review?(dbaron)
Comment on attachment 808504 [details] [diff] [review]
rename

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

Looks good to me.
Attachment #808504 - Flags: review?(fred.wang) → review+
Comment on attachment 808504 [details] [diff] [review]
rename

Asking review to roc for the renaming part, in case David is busy.
Attachment #808504 - Flags: review?(roc)
Attachment #808504 - Flags: review?(dbaron)
No new tests since it's essentially some renaming and a trivial fix to nsMathMLTokenFrame::ForceTrimChildTextFrames.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/2933d4d279c5
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.