Summary: | Replace static_casts with to* functions - Part 2 | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Abhishek Arya <inferno> | ||||||||||
Component: | WebKit Misc. | Assignee: | Abhishek Arya <inferno> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, pdr, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 112097 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Abhishek Arya
2013-03-11 15:05:08 PDT
Created attachment 192570 [details]
Patch
Comment on attachment 192570 [details]
Patch
Can you add a bit more to the ChangeLog so people aren't confused when they see this in a few years? Just a small summary of what we're doing here.
LGTM otherwise.
Created attachment 192579 [details]
Patch
Comment on attachment 192579 [details] Patch Clearing flags on attachment: 192579 Committed r145462: <http://trac.webkit.org/changeset/145462> All reviewed patches have been landed. Closing bug. Re-opened since this is blocked by bug 112097 Created attachment 192629 [details]
Patch
Loads of toElement() crashes in WebCore::ApplyStyleCommand::applyBlockStyle http://build.webkit.org/results/Apple%20Lion%20Debug%20WK1%20(Tests)/r145468%20(7365)/results.html (In reply to comment #8) > Loads of toElement() crashes in WebCore::ApplyStyleCommand::applyBlockStyle > > http://build.webkit.org/results/Apple%20Lion%20Debug%20WK1%20(Tests)/r145468%20(7365)/results.html That's a false positive. We should be using toContainerNode instead. Comment on attachment 192629 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192629&action=review > Source/WebCore/editing/ApplyStyleCommand.cpp:294 > - startRange = TextIterator::rangeFromLocationAndLength(static_cast<Element*>(scope), startIndex, 0, true); > - endRange = TextIterator::rangeFromLocationAndLength(static_cast<Element*>(scope), endIndex, 0, true); > + startRange = TextIterator::rangeFromLocationAndLength(toElement(scope), startIndex, 0, true); > + endRange = TextIterator::rangeFromLocationAndLength(toElement(scope), endIndex, 0, true); Please use toContainerNode here. scope can be Document. (In reply to comment #10) > (From update of attachment 192629 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=192629&action=review > > > Source/WebCore/editing/ApplyStyleCommand.cpp:294 > > - startRange = TextIterator::rangeFromLocationAndLength(static_cast<Element*>(scope), startIndex, 0, true); > > - endRange = TextIterator::rangeFromLocationAndLength(static_cast<Element*>(scope), endIndex, 0, true); > > + startRange = TextIterator::rangeFromLocationAndLength(toElement(scope), startIndex, 0, true); > > + endRange = TextIterator::rangeFromLocationAndLength(toElement(scope), endIndex, 0, true); > > Please use toContainerNode here. scope can be Document. Thanks a lot Ryosuke for confirming. I don't understand why cr-linux-debug didnt shout at me :(. Created attachment 192643 [details]
Patch
(In reply to comment #11) > > Thanks a lot Ryosuke for confirming. I don't understand why cr-linux-debug didnt shout at me :(. cr-linux-debug doesn't run tests. Comment on attachment 192643 [details] Patch Attachment 192643 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/17174047 New failing tests: platform/mac/fast/speechsynthesis/speech-synthesis-speak.html Lets me check these out locally, they might be real bugs. Regressions: Unexpected crashes (2) editing/selection/selection-modify-crash.html [ Crash ] platform/mac/fast/speechsynthesis/speech-synthesis-speak.html [ Crash ] Comment on attachment 192643 [details]
Patch
Nothing crashes locally on mac, going cq+.
Comment on attachment 192643 [details] Patch Clearing flags on attachment: 192643 Committed r145562: <http://trac.webkit.org/changeset/145562> All reviewed patches have been landed. Closing bug. |