Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3)

Issue 1121633004: Revert Range firstNode/pastLastNode() refactoring. (Closed)

Created:
5 years, 7 months ago by sof
Modified:
5 years, 7 months ago
Reviewers:
tkent, haraken, yosin_UTC9
CC:
blink-reviews, blink-reviews-dom_chromium.org, dglazkov+blink, eae+blinkwatch, rwlbuis
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Revert Range firstNode/pastLastNode() refactoring. The calculation of these two end points cannot in general be done by way of anchor offsetting of the start/end node -- such an anchor may not exist. Hence, revert their implementations back to how they were prior to r191892. The functionality that r191892 added to Position has separate uses, and is kept. R=haraken BUG=484103 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=194918

Patch Set 1 #

Patch Set 2 : Revert back to previous versions of firstNode/pastLastNode + add test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -4 lines) Patch
A LayoutTests/fast/dom/Range/range-toString-non-anchor-no-crash.html View 1 1 chunk +16 lines, -0 lines 0 comments Download
A + LayoutTests/fast/dom/Range/range-toString-non-anchor-no-crash-expected.txt View 1 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/dom/Range.cpp View 1 2 chunks +12 lines, -2 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
sof
Please take a look. A regression it'd be good to address before M44 is cut.
5 years, 7 months ago (2015-05-05 08:26:21 UTC) #2
haraken
LGTM This should be reviewed by yosin@, but given that Japan is a holiday season ...
5 years, 7 months ago (2015-05-05 10:36:53 UTC) #3
sof
On 2015/05/05 10:36:53, haraken wrote: > LGTM > > This should be reviewed by yosin@, ...
5 years, 7 months ago (2015-05-05 11:05:59 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1121633004/20001
5 years, 7 months ago (2015-05-05 11:06:31 UTC) #6
commit-bot: I haz the power
5 years, 7 months ago (2015-05-05 11:10:02 UTC) #7
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194918

Powered by Google App Engine
This is Rietveld 408576698