Fix links are not shown for srcset in view-source

Fix links are not shown for srcset in view-source

added specific handler for srcset in the view-source

BUG=633336
Committed: https://crrev.com/62f9562970add03544cf22ebe95ac38b5e2922c9
Cr-Commit-Position: refs/heads/master@{#440071}

[wanchang, 2016-12-20 08:09:18]

Description was changed from

==========
Fix links are not shown for srcset in view-source

added specific handler for srcset in the view-source

BUG=633336
==========

to

==========
Fix links are not shown for srcset in view-source

added specific handler for srcset in the view-source

BUG=633336
==========

[wanchang, 2016-12-20 08:09:19]

wanchang.ryu@lge.com changed reviewers:
+ skobes@chromium.org

[wanchang, 2016-12-20 08:12:51]

The current implementation does not consider srcset attribute as link in
view-source since HTMLViewSourceDocument is missing handling this attribute.

I upload the patch to fix the issue
https://bugs.chromium.org/p/chromium/issues/detail?id=633336.

Please review my patch and any comments will be appreciated.
Thanks

[wanchang, 2016-12-20 08:32:38]

wanchang.ryu@lge.com changed reviewers:
+ abarth@chromium.org

[wanchang, 2016-12-20 08:40:55]

wanchang.ryu@lge.com changed reviewers:
+ pfeldman@chromium.org

[skobes, 2016-12-20 23:54:45]

Can you add a test?

[wanchang, 2016-12-21 00:47:20]

On 2016/12/20 23:54:45, skobes wrote:
> Can you add a test?

Dear skobes,

Could you help me where I should add a test ?
I like to add a test on third_party/WebKit/LayoutTests/fast/frames/viewsource.
Is it okay ?

Thanks

[skobes, 2016-12-21 01:05:56]

On 2016/12/21 00:47:20, wanchang.ryu wrote:
> Could you help me where I should add a test ?
> I like to add a test on third_party/WebKit/LayoutTests/fast/frames/viewsource.
> Is it okay ?

Yes that looks like a good place.

[wanchang, 2016-12-21 01:18:04]

On 2016/12/21 01:05:56, skobes wrote:
> On 2016/12/21 00:47:20, wanchang.ryu wrote:
> > Could you help me where I should add a test ?
> > I like to add a test on
third_party/WebKit/LayoutTests/fast/frames/viewsource.
> > Is it okay ?
> 
> Yes that looks like a good place.

I added a test.
Please review the patch, any comments will be appreciated.
Thanks

[skobes, 2016-12-21 01:35:06]

lgtm with nit

https://codereview.chromium.org/2589123002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLViewSourceDocument.cpp (right):

https://codereview.chromium.org/2589123002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLViewSourceDocument.cpp:332:
addText(srclist[i],"html-attribute-value");
put a space after ','

[wanchang, 2016-12-21 01:49:02]

On 2016/12/21 01:35:06, skobes wrote:
> lgtm with nit
> 
>
https://codereview.chromium.org/2589123002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/html/HTMLViewSourceDocument.cpp (right):
> 
>
https://codereview.chromium.org/2589123002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/html/HTMLViewSourceDocument.cpp:332:
> addText(srclist[i],"html-attribute-value");
> put a space after ','

I add a space after ',' on the each place.
Thanks

[wanchang, 2016-12-21 02:13:27]

On 2016/12/21 01:49:02, wanchang.ryu wrote:
> On 2016/12/21 01:35:06, skobes wrote:
> > lgtm with nit
> > 
> >
>
https://codereview.chromium.org/2589123002/diff/20001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/html/HTMLViewSourceDocument.cpp (right):
> > 
> >
>
https://codereview.chromium.org/2589123002/diff/20001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/html/HTMLViewSourceDocument.cpp:332:
> > addText(srclist[i],"html-attribute-value");
> > put a space after ','
> 
> I add a space after ',' on the each place.
> Thanks

Dear skobes,
I uploaded new patchset, Should I wait for another your lgtm comment to commit
this patch ?
Thanks

[wanchang, 2016-12-21 08:59:24]

The CQ bit was checked by wanchang.ryu@lge.com

[wanchang, 2016-12-21 08:59:24]

The patchset sent to the CQ was uploaded after l-g-t-m from skobes@chromium.org
Link to the patchset: https://codereview.chromium.org/2589123002/#ps40001
(title: "Fix links are not shown for srcset in view-source")

[commit-bot: I haz the power, 2016-12-21 08:59:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-21 10:56:52]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1482310764555580,
"parent_rev": "9b029d9e022247936b080c8016db835592da197a", "commit_rev":
"d6c6ab133858fc20a450fe7922a06eda413fa2ce"}

[commit-bot: I haz the power, 2016-12-21 10:57:18]

Description was changed from

==========
Fix links are not shown for srcset in view-source

added specific handler for srcset in the view-source

BUG=633336
==========

to

==========
Fix links are not shown for srcset in view-source

added specific handler for srcset in the view-source

BUG=633336

Review-Url: https://codereview.chromium.org/2589123002
==========

[commit-bot: I haz the power, 2016-12-21 10:57:19]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-21 10:59:45]

Description was changed from

==========
Fix links are not shown for srcset in view-source

added specific handler for srcset in the view-source

BUG=633336

Review-Url: https://codereview.chromium.org/2589123002
==========

to

==========
Fix links are not shown for srcset in view-source

added specific handler for srcset in the view-source

BUG=633336

Committed: https://crrev.com/62f9562970add03544cf22ebe95ac38b5e2922c9
Cr-Commit-Position: refs/heads/master@{#440071}
==========

[commit-bot: I haz the power, 2016-12-21 10:59:47]

Patchset 3 (id:??) landed as
https://crrev.com/62f9562970add03544cf22ebe95ac38b5e2922c9
Cr-Commit-Position: refs/heads/master@{#440071}

[skobes, 2016-12-21 17:42:39]

On 2016/12/21 02:13:27, wanchang wrote:
> Dear skobes,
> I uploaded new patchset, Should I wait for another your lgtm comment to commit
> this patch ?

If you get "lgtm with nits" you can submit after addressing the nits.  No need
to wait. :)

[wanchang, 2016-12-21 23:26:10]

On 2016/12/21 17:42:39, skobes wrote:
> On 2016/12/21 02:13:27, wanchang wrote:
> > Dear skobes,
> > I uploaded new patchset, Should I wait for another your lgtm comment to
commit
> > this patch ?
> 
> If you get "lgtm with nits" you can submit after addressing the nits.  No need
> to wait. :)

Thanks for your comments.