Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2755123002:
Fix Find in Page on iOS 10.3. (Closed)
|
Created:
3 years, 9 months ago by stkhapugin Modified:
3 years, 9 months ago CC:
chromium-reviews, ios-reviews+chrome_chromium.org, ios-reviews_chromium.org, pkl (ping after 24h if needed), noyau+watch_chromium.org, marq+watch_chromium.org, sdefresne+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix Find in Page on iOS 10.3.
In iOS 10.3, an error is generated when trying to find in page on a page
with iframe:
SecurityError (DOM Exception 18): Blocked a frame with origin "https://w
ww.reddit.com" from accessing a frame with origin "https://www.redditmed
ia.com". Protocols, domains, and ports must match.
As a result, the search results are not highlighted on the page.
The fix is wrapping the offending line in try/catch.
BUG=702566
TEST=On iOS 10.3 device, open cnn.com, select tools>Find in Page" and
type any string that is present on the page. The string should be found
and highglighted, and the number of results should be correct, and
next/previous buttons should scroll the page to the next/prev result.
Review-Url: https://codereview.chromium.org/2755123002
Cr-Commit-Position: refs/heads/master@{#457845}
Committed: https://chromium.googlesource.com/chromium/src/+/2991f43a94bbdb2eb798a8f256b67b5b3865f129
Patch Set 1 #
Total comments: 4
Patch Set 2 : comment added #Patch Set 3 : space #Messages
Total messages: 29 (16 generated)
The CQ bit was checked by stkhapugin@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 ========== Fix Find in Page on iOS 10.3. In iOS 10.3, an error is generated when trying to find in page on a page with iframe: SecurityError (DOM Exception 18): Blocked a frame with origin "https://w ww.reddit.com" from accessing a frame with origin "https://www.redditmed ia.com". Protocols, domains, and ports must match. As a result, the search results are not highlighted on the page. The fix is wrapping the offending line in try/catch. BUG=702566 TEST=On iOS 10.3 device, open cnn.com, select tools>Find in Page" and type any string that is present on the page. The string should be found and highglighted, and the number of results should be correct, and next/previous buttons should scroll the page to the next/prev result. ========== to ========== Fix Find in Page on iOS 10.3. In iOS 10.3, an error is generated when trying to find in page on a page with iframe: SecurityError (DOM Exception 18): Blocked a frame with origin "https://w ww.reddit.com" from accessing a frame with origin "https://www.redditmed ia.com". Protocols, domains, and ports must match. As a result, the search results are not highlighted on the page. The fix is wrapping the offending line in try/catch. BUG=702566 TEST=On iOS 10.3 device, open cnn.com, select tools>Find in Page" and type any string that is present on the page. The string should be found and highglighted, and the number of results should be correct, and next/previous buttons should scroll the page to the next/prev result. ==========
stkhapugin@chromium.org changed reviewers: + rohitrao@chromium.org
PTAL
stkhapugin@chromium.org changed reviewers: + justincohen@chromium.org - rohitrao@chromium.org
lgtm https://codereview.chromium.org/2755123002/diff/1/ios/chrome/browser/find_in_... File ios/chrome/browser/find_in_page/resources/find_in_page.js (right): https://codereview.chromium.org/2755123002/diff/1/ios/chrome/browser/find_in_... ios/chrome/browser/find_in_page/resources/find_in_page.js:956: try{ nit space
https://codereview.chromium.org/2755123002/diff/1/ios/chrome/browser/find_in_... File ios/chrome/browser/find_in_page/resources/find_in_page.js (right): https://codereview.chromium.org/2755123002/diff/1/ios/chrome/browser/find_in_... ios/chrome/browser/find_in_page/resources/find_in_page.js:961: } catch (e) { probably best to explain what we expect to catch here, and why it's ok to ignore it.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
thanks ptal https://codereview.chromium.org/2755123002/diff/1/ios/chrome/browser/find_in_... File ios/chrome/browser/find_in_page/resources/find_in_page.js (right): https://codereview.chromium.org/2755123002/diff/1/ios/chrome/browser/find_in_... ios/chrome/browser/find_in_page/resources/find_in_page.js:956: try{ On 2017/03/17 15:05:34, justincohen (OOO until Mar 20) wrote: > nit space Done. https://codereview.chromium.org/2755123002/diff/1/ios/chrome/browser/find_in_... ios/chrome/browser/find_in_page/resources/find_in_page.js:961: } catch (e) { On 2017/03/17 15:06:25, justincohen (OOO until Mar 20) wrote: > probably best to explain what we expect to catch here, and why it's ok to ignore > it. Done.
still lgtm
The CQ bit was checked by stkhapugin@chromium.org
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
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by stkhapugin@chromium.org
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
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
stkhapugin@chromium.org changed reviewers: + sdefresne@chromium.org
+sdefresne for OWNERS, PTAL
rohitrao@chromium.org changed reviewers: + rohitrao@chromium.org
lgtm
The CQ bit was checked by stkhapugin@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": 40001, "attempt_start_ts": 1489777006483500,
"parent_rev": "8fca31274c6a41144b4f4ee63496c3cc18f15f54", "commit_rev":
"2991f43a94bbdb2eb798a8f256b67b5b3865f129"}
Message was sent while issue was closed.
Description was changed from ========== Fix Find in Page on iOS 10.3. In iOS 10.3, an error is generated when trying to find in page on a page with iframe: SecurityError (DOM Exception 18): Blocked a frame with origin "https://w ww.reddit.com" from accessing a frame with origin "https://www.redditmed ia.com". Protocols, domains, and ports must match. As a result, the search results are not highlighted on the page. The fix is wrapping the offending line in try/catch. BUG=702566 TEST=On iOS 10.3 device, open cnn.com, select tools>Find in Page" and type any string that is present on the page. The string should be found and highglighted, and the number of results should be correct, and next/previous buttons should scroll the page to the next/prev result. ========== to ========== Fix Find in Page on iOS 10.3. In iOS 10.3, an error is generated when trying to find in page on a page with iframe: SecurityError (DOM Exception 18): Blocked a frame with origin "https://w ww.reddit.com" from accessing a frame with origin "https://www.redditmed ia.com". Protocols, domains, and ports must match. As a result, the search results are not highlighted on the page. The fix is wrapping the offending line in try/catch. BUG=702566 TEST=On iOS 10.3 device, open cnn.com, select tools>Find in Page" and type any string that is present on the page. The string should be found and highglighted, and the number of results should be correct, and next/previous buttons should scroll the page to the next/prev result. Review-Url: https://codereview.chromium.org/2755123002 Cr-Commit-Position: refs/heads/master@{#457845} Committed: https://chromium.googlesource.com/chromium/src/+/2991f43a94bbdb2eb798a8f256b6... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/2991f43a94bbdb2eb798a8f256b6... |
