<msubsup> setNeedsLayout() correction
Created attachment 129376 [details]
Comment on attachment 129376 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=129376&action=review
> + superscriptWrapper->setNeedsLayout(true, false); in the last patch actually needs to be
> + superscriptWrapper->setNeedsLayout(true); in order to mark its parent, m_scripts, as
> + needing layout also.
I would rather see a patch with your parent marked in this case. As I told you calling setNeedsLayout(true, true) during layout is really something you don't want to do. That is unless you want to open a security can of worms.
> + No new tests. It's actually difficult to test the need for this with the current code,
I would really love to see a test for 2 reasons:
* this would help your future refactorings.
* this will give some confidence that the code is properly tested.
Created attachment 129467 [details]
I changed the code to mark the m_scripts parent.
As discussed in private e-mail, the existing tests should cover this fix, except that the current code has a lot of extra setNeedsLayout() calls, so the tests pass anyway. It's hard to come up with a test that wouldn't pass now, but there may be one. We could wait to land this change until we're sure it's needed (the existing tests will start to fail), but that will complicate a future patch, and a potential bug will remain in the meantime.
Attachment 129467 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:8: Line contains tab character. [whitespace/tab] 
Total errors found: 1 in 2 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 129467 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=129467&action=review
>> + m_scripts->setNeedsLayout(true, false); needs to be added to the fix for bug 79274.
> Line contains tab character. [whitespace/tab] 
You will not be able to land this. I will fix and submit the patch for landing.
Created attachment 129488 [details]
Patch for landing
Comment on attachment 129488 [details]
Patch for landing
Clearing flags on attachment: 129488
Committed r109286: <http://trac.webkit.org/changeset/109286>
All reviewed patches have been landed. Closing bug.