WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
90807
WebSurroundingText layout tests should use the same code path as the rest of the feature
https://bugs.webkit.org/show_bug.cgi?id=90807
Summary
WebSurroundingText layout tests should use the same code path as the rest of ...
Leandro Graciá Gil
Reported
2012-07-09 11:39:40 PDT
Currently WebSurroundingText has a custom initialize method based on node offsets that is used only by the LayoutTestController. Instead of this we should make the tests work with coordinates and use the same code path as the rest of the feature.
Attachments
Patch
(12.63 KB, patch)
2012-07-10 07:11 PDT
,
Leandro Graciá Gil
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Leandro Graciá Gil
Comment 1
2012-07-10 07:11:43 PDT
Created
attachment 151455
[details]
Patch
Leandro Graciá Gil
Comment 2
2012-07-10 07:17:19 PDT
Comment on
attachment 151455
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=151455&action=review
> Source/WebKit/chromium/src/WebSurroundingText.cpp:43 > +void WebSurroundingText::initialize(const WebHitTestResult& hitTestResult, size_t maxLength)
This method is currently in use by Chromium. However, we can switch to the new one and remove this with a 3-sided patch.
> LayoutTests/platform/chromium/editing/surrounding-text/surrounding-text.html:-36 > - shouldBeEqualToString('surroundingText(\'<button>.</button>12345<p id="here">6789 12345</p>6789<button>.</button>\', 100, 5)', '');
Invalid offsets throw a JS exception in findOffsetCoordinates now.
WebKit Review Bot
Comment 3
2012-07-10 07:19:44 PDT
Please wait for approval from
abarth@webkit.org
,
dglazkov@chromium.org
,
fishd@chromium.org
,
jamesr@chromium.org
or
tkent@chromium.org
before submitting, as this patch contains changes to the Chromium public API. See also
https://trac.webkit.org/wiki/ChromiumWebKitAPI
.
Adam Barth
Comment 4
2012-07-10 08:08:50 PDT
Comment on
attachment 151455
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=151455&action=review
>> Source/WebKit/chromium/src/WebSurroundingText.cpp:43 >> +void WebSurroundingText::initialize(const WebHitTestResult& hitTestResult, size_t maxLength) > > This method is currently in use by Chromium. However, we can switch to the new one and remove this with a 3-sided patch.
If you like, you can add a FIXME comment to the public header to this effect.
WebKit Review Bot
Comment 5
2012-07-10 09:01:44 PDT
Comment on
attachment 151455
[details]
Patch Clearing flags on attachment: 151455 Committed
r122225
: <
http://trac.webkit.org/changeset/122225
>
WebKit Review Bot
Comment 6
2012-07-10 09:01:49 PDT
All reviewed patches have been landed. Closing bug.
Leandro Graciá Gil
Comment 7
2012-07-10 09:37:20 PDT
Committed
r122230
: <
http://trac.webkit.org/changeset/122230
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug