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

Issue 661883003: add option for original domain (Closed)

Created:
6 years, 2 months ago by kuan
Modified:
6 years, 1 month ago
Reviewers:
cjhopman
Base URL:
https://code.google.com/p/dom-distiller/@master
Visibility:
Public.

Description

add option for original domain - this is part of the implementation of scoring mechanism for next/prev page links. - scoring framework uses files, instead of live urls, when calling distiller. detection of next/prev page links expects the same domain for current page and paging links, which isn't the case when current page is a file://. - this is rectified by having an option to specify the original domain associated with the current page if it's a file://. - detection algorithm uses this original domain when determining location host. - fixed and added tests. BUG=425952 R=cjhopman@chromium.org Committed: 62f5e927e0996d0d357d95e4131495beac4973e6

Patch Set 1 #

Patch Set 2 : upd comment #

Total comments: 2

Patch Set 3 : addressed comment #

Patch Set 4 : rm empty line #

Unified diffs Side-by-side diffs Delta from patch set Stats (+54 lines, -34 lines) Patch
M proto/dom_distiller.proto View 1 1 chunk +4 lines, -0 lines 0 comments Download
M src/com/dom_distiller/client/DomDistiller.java View 1 chunk +2 lines, -1 line 0 comments Download
M src/com/dom_distiller/client/PagingLinksFinder.java View 4 chunks +17 lines, -11 lines 0 comments Download
M test/com/dom_distiller/client/PagingLinksFinderTest.java View 1 2 3 9 chunks +31 lines, -22 lines 0 comments Download

Messages

Total messages: 8 (3 generated)
kuan
6 years, 2 months ago (2014-10-23 23:21:11 UTC) #2
cjhopman
https://codereview.chromium.org/661883003/diff/20001/test/com/dom_distiller/client/PagingLinksFinderTest.java File test/com/dom_distiller/client/PagingLinksFinderTest.java (right): https://codereview.chromium.org/661883003/diff/20001/test/com/dom_distiller/client/PagingLinksFinderTest.java#newcode154 test/com/dom_distiller/client/PagingLinksFinderTest.java:154: Can we add a simple test that uses a ...
6 years, 1 month ago (2014-10-24 22:13:47 UTC) #3
kuan
i've added the test in patches set 2 and 3. ptal. thx. https://codereview.chromium.org/661883003/diff/20001/test/com/dom_distiller/client/PagingLinksFinderTest.java File test/com/dom_distiller/client/PagingLinksFinderTest.java ...
6 years, 1 month ago (2014-10-24 23:07:11 UTC) #4
cjhopman
lgtm
6 years, 1 month ago (2014-10-28 23:09:05 UTC) #6
kuan
6 years, 1 month ago (2014-10-29 02:54:47 UTC) #8
Message was sent while issue was closed.
Committed patchset #4 (id:60001) manually as
62f5e927e0996d0d357d95e4131495beac4973e6 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698