|
|
DescriptionUse NodeAsRangeFirstNode() as Range::FirstNode() and
NodeAsRangePastLastNode() as Range::PastLastNode().
This was introduced once 2 years ago:
https://codereview.chromium.org/1000533004
but reverted of crash:
https://codereview.chromium.org/1121633004
Now original code won't crash because {start/end}Position() doesn't return
Null Position since:
https://codereview.chromium.org/2812733003
TEST=LayoutTests/fast/dom/Range/range-toString-non-anchor-no-crash.html
Review-Url: https://codereview.chromium.org/2818643003
Cr-Commit-Position: refs/heads/master@{#464358}
Committed: https://chromium.googlesource.com/chromium/src/+/4074f42f1c8360b2f429549b8819f4d9f604ebd4
Patch Set 1 #
Total comments: 4
Patch Set 2 : update #Messages
Total messages: 26 (18 generated)
The CQ bit was checked by yoichio@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Use Position in range TEST= ========== to ========== Use Position in range This was introduced once 2 years ago: https://codereview.chromium.org/1000533004 but reverted of crash: https://codereview.chromium.org/1121633004 Now original code won't crash because {start/end}Position() doesn't return Null Position since: https://codereview.chromium.org/2812733003 TEST=LayoutTests/fast/dom/Range/range-toString-non-anchor-no-crash.html ==========
Description was changed from ========== Use Position in range This was introduced once 2 years ago: https://codereview.chromium.org/1000533004 but reverted of crash: https://codereview.chromium.org/1121633004 Now original code won't crash because {start/end}Position() doesn't return Null Position since: https://codereview.chromium.org/2812733003 TEST=LayoutTests/fast/dom/Range/range-toString-non-anchor-no-crash.html ========== to ========== Use NodeAsRangeFirstNode as Range::FirstNode again. This was introduced once 2 years ago: https://codereview.chromium.org/1000533004 but reverted of crash: https://codereview.chromium.org/1121633004 Now original code won't crash because {start/end}Position() doesn't return Null Position since: https://codereview.chromium.org/2812733003 TEST=LayoutTests/fast/dom/Range/range-toString-non-anchor-no-crash.html ==========
Description was changed from ========== Use NodeAsRangeFirstNode as Range::FirstNode again. This was introduced once 2 years ago: https://codereview.chromium.org/1000533004 but reverted of crash: https://codereview.chromium.org/1121633004 Now original code won't crash because {start/end}Position() doesn't return Null Position since: https://codereview.chromium.org/2812733003 TEST=LayoutTests/fast/dom/Range/range-toString-non-anchor-no-crash.html ========== to ========== Use NodeAsRangeFirstNode() as Range::FirstNode() and NodeAsRangePastLastNode() as Range::PastLastNode(). This was introduced once 2 years ago: https://codereview.chromium.org/1000533004 but reverted of crash: https://codereview.chromium.org/1121633004 Now original code won't crash because {start/end}Position() doesn't return Null Position since: https://codereview.chromium.org/2812733003 TEST=LayoutTests/fast/dom/Range/range-toString-non-anchor-no-crash.html ==========
yoichio@chromium.org changed reviewers: + yosin@chromium.org
The CQ bit was checked by yoichio@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2818643003/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/dom/Range.cpp (right): https://codereview.chromium.org/2818643003/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/dom/Range.cpp:1436: return StartPosition().ToOffsetInAnchor().NodeAsRangeFirstNode(); We don't need to use |ToOffsetInAnchor()| here. |Position::NodeAsRangeFirstNode()| does so. BTW, we would like to call |ToOffsetInAnchor()| in |Position::NodeAsRangeFirstNode()|. https://codereview.chromium.org/2818643003/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/dom/Range.cpp:1440: return EndPosition().ToOffsetInAnchor().NodeAsRangePastLastNode(); We don't need to use |ToOffsetInAnchor()| here. |Position::NodeAsRangePastLastNode()| does so. BTW, we would like to call |ToOffsetInAnchor()| in |Position::NodeAsRangePastLastNode()|.
The CQ bit was checked by yoichio@chromium.org to run a CQ dry run
https://codereview.chromium.org/2818643003/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/dom/Range.cpp (right): https://codereview.chromium.org/2818643003/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/dom/Range.cpp:1436: return StartPosition().ToOffsetInAnchor().NodeAsRangeFirstNode(); On 2017/04/13 05:45:45, yosin_UTC9 wrote: > We don't need to use |ToOffsetInAnchor()| here. > |Position::NodeAsRangeFirstNode()| does so. > BTW, we would like to call |ToOffsetInAnchor()| in > |Position::NodeAsRangeFirstNode()|. Done. https://codereview.chromium.org/2818643003/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/dom/Range.cpp:1440: return EndPosition().ToOffsetInAnchor().NodeAsRangePastLastNode(); On 2017/04/13 05:45:45, yosin_UTC9 wrote: > We don't need to use |ToOffsetInAnchor()| here. > |Position::NodeAsRangePastLastNode()| does so. > BTW, we would like to call |ToOffsetInAnchor()| in > |Position::NodeAsRangePastLastNode()|. Done.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
yosin@chromium.org changed reviewers: + tkent@chromium.org
lgtm +tkent@ for OWNERS review
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by yoichio@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492075584178510, "parent_rev": "a15de7e2276f1ccaf8392b4268002f2979a246a5", "commit_rev": "4074f42f1c8360b2f429549b8819f4d9f604ebd4"}
Message was sent while issue was closed.
Description was changed from ========== Use NodeAsRangeFirstNode() as Range::FirstNode() and NodeAsRangePastLastNode() as Range::PastLastNode(). This was introduced once 2 years ago: https://codereview.chromium.org/1000533004 but reverted of crash: https://codereview.chromium.org/1121633004 Now original code won't crash because {start/end}Position() doesn't return Null Position since: https://codereview.chromium.org/2812733003 TEST=LayoutTests/fast/dom/Range/range-toString-non-anchor-no-crash.html ========== to ========== Use NodeAsRangeFirstNode() as Range::FirstNode() and NodeAsRangePastLastNode() as Range::PastLastNode(). This was introduced once 2 years ago: https://codereview.chromium.org/1000533004 but reverted of crash: https://codereview.chromium.org/1121633004 Now original code won't crash because {start/end}Position() doesn't return Null Position since: https://codereview.chromium.org/2812733003 TEST=LayoutTests/fast/dom/Range/range-toString-non-anchor-no-crash.html Review-Url: https://codereview.chromium.org/2818643003 Cr-Commit-Position: refs/heads/master@{#464358} Committed: https://chromium.googlesource.com/chromium/src/+/4074f42f1c8360b2f429549b8819... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/4074f42f1c8360b2f429549b8819... |