DevTools: Fix a regression that double-clicking an item in the Network tab doesn't open the item in…

DevTools: Fix a regression that double-clicking an item in the Network tab doesn't open the item in a new tab.

ViewportDataGrid._update() has the following code:
    tBody.insertBefore(element, previousElement.nextSibling);
where |previousElement.nextSibling| can be |element|.

The code did nothing before [1], but we remove |element| from |tBody| and add it
to |tBody| again after [1]. Because the code is executed in 'mousedown' event
handler, 'dblclick' event for |element| wasn't dispatched.

This CL updates ViewportDataGrid so that it skips to call insertBefore() if
|element| is |previousElement.nextSibling|.


[1] https://chromium.googlesource.com/chromium/src/+/1c12127b721f833f37b81553e19b8eb0392da651

BUG=716583
Review-Url: https://codereview.chromium.org/2869643002
Cr-Commit-Position: refs/heads/master@{#470833}
Committed: https://chromium.googlesource.com/chromium/src/+/6d44fde7f95c979cd78698142389b6fe4fa8916f

[tkent, 2017-05-08 04:10:21]

The CQ bit was checked by tkent@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-08 04:10:37]

Dry run: CQ is trying da patch.

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

[tkent, 2017-05-08 04:10:57]

Description was changed from

==========
DevTools: Fix a regression that double-clicking an item in the Network tab
doesn't open the item in a new tab.

ViewportDataGrid._update() has the following code:
    tBody.insertBefore(element, previousElement.nextSibling);
where |previousElement.nextSibling| is |element|.

The code did nothing before [1], but we remove |element| from |tBody| and add it
to |tBody| again after [1]. Because the code is executed in mousedown event
handler, 'dblclick' event for |element| wasn't dispatched.

This CL makes Element.insertBefore() and appendChild() compatible with the
previous behavior by updating Widget.js.

[1]
https://chromium.googlesource.com/chromium/src/+/1c12127b721f833f37b81553e19b...

BUG=716583
==========

to

==========
DevTools: Fix a regression that double-clicking an item in the Network tab
doesn't open the item in a new tab.

ViewportDataGrid._update() has the following code:
    tBody.insertBefore(element, previousElement.nextSibling);
where |previousElement.nextSibling| can be |element|.

The code did nothing before [1], but we remove |element| from |tBody| and add it
to |tBody| again after [1]. Because the code is executed in 'mousedown' event
handler, 'dblclick' event for |element| wasn't dispatched.

This CL makes Element.insertBefore() and appendChild() compatible with the
previous behavior by updating Widget.js.

[1]
https://chromium.googlesource.com/chromium/src/+/1c12127b721f833f37b81553e19b...

BUG=716583
==========

[commit-bot: I haz the power, 2017-05-08 04:54:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-08 04:54:53]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tkent, 2017-05-08 07:33:51]

tkent@chromium.org changed reviewers:
+ lushnikov@chromium.org

[tkent, 2017-05-08 07:33:52]

lushnikov@, would you review this, or would you route this to an appropriate
reviewer please?

* I'm not sure this approach is the best. insertBefore() and appendChild() in
DevTools will have behavior different from the DOM standard.  We might want to
move the code to ViewportDataGrid.

* I'm not sure how to test this.

[lushnikov, 2017-05-08 19:51:03]

On 2017/05/08 07:33:52, tkent wrote:
> lushnikov@, would you review this, or would you route this to an appropriate
> reviewer please?
> 
> * I'm not sure this approach is the best. insertBefore() and appendChild() in
> DevTools will have behavior different from the DOM standard.  We might want to
> move the code to ViewportDataGrid.

I agree with your point, let's fix this locally in the ViewportDataGrid.
Our overrides for appendChild/insertBefore should not do anything but sanity
checks
for the widget system.

[tkent, 2017-05-09 06:48:25]

The CQ bit was checked by tkent@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-09 06:49:03]

Dry run: 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, 2017-05-09 09:21:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-09 09:21:06]

Dry run: This issue passed the CQ dry run.

[tkent, 2017-05-09 13:25:38]

On 2017/05/08 at 19:51:03, lushnikov wrote:
> > * I'm not sure this approach is the best. insertBefore() and appendChild()
in
> > DevTools will have behavior different from the DOM standard.  We might want
to
> > move the code to ViewportDataGrid.
> 
> I agree with your point, let's fix this locally in the ViewportDataGrid.
> Our overrides for appendChild/insertBefore should not do anything but sanity
checks
> for the widget system.

ok, updated the patch.

[tkent, 2017-05-09 13:27:12]

Description was changed from

==========
DevTools: Fix a regression that double-clicking an item in the Network tab
doesn't open the item in a new tab.

ViewportDataGrid._update() has the following code:
    tBody.insertBefore(element, previousElement.nextSibling);
where |previousElement.nextSibling| can be |element|.

The code did nothing before [1], but we remove |element| from |tBody| and add it
to |tBody| again after [1]. Because the code is executed in 'mousedown' event
handler, 'dblclick' event for |element| wasn't dispatched.

This CL makes Element.insertBefore() and appendChild() compatible with the
previous behavior by updating Widget.js.

[1]
https://chromium.googlesource.com/chromium/src/+/1c12127b721f833f37b81553e19b...

BUG=716583
==========

to

==========
DevTools: Fix a regression that double-clicking an item in the Network tab
doesn't open the item in a new tab.

ViewportDataGrid._update() has the following code:
    tBody.insertBefore(element, previousElement.nextSibling);
where |previousElement.nextSibling| can be |element|.

The code did nothing before [1], but we remove |element| from |tBody| and add it
to |tBody| again after [1]. Because the code is executed in 'mousedown' event
handler, 'dblclick' event for |element| wasn't dispatched.

This CL updates ViewportDataGrid so that it skips to call insertBefore() if
|element| is |previousElement.nextSibling|.


[1]
https://chromium.googlesource.com/chromium/src/+/1c12127b721f833f37b81553e19b...

BUG=716583
==========

[lushnikov, 2017-05-09 18:33:00]

lushnikov@chromium.org changed reviewers:
+ dgozman@chromium.org

[lushnikov, 2017-05-09 18:33:01]

lgtm, thanks

cc'ing Dmitry so that he's aware about this overall

[tkent, 2017-05-10 22:45:58]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2017-05-10 22:46:47]

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, 2017-05-11 00:53:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-11 00:53:05]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tkent, 2017-05-11 04:17:37]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2017-05-11 04:19:06]

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, 2017-05-11 05:55:24]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494476257627360,
"parent_rev": "5727d41e60da68273c3a583e42ff447f47b26d93", "commit_rev":
"6d44fde7f95c979cd78698142389b6fe4fa8916f"}

[commit-bot: I haz the power, 2017-05-11 05:55:38]

Description was changed from

==========
DevTools: Fix a regression that double-clicking an item in the Network tab
doesn't open the item in a new tab.

ViewportDataGrid._update() has the following code:
    tBody.insertBefore(element, previousElement.nextSibling);
where |previousElement.nextSibling| can be |element|.

The code did nothing before [1], but we remove |element| from |tBody| and add it
to |tBody| again after [1]. Because the code is executed in 'mousedown' event
handler, 'dblclick' event for |element| wasn't dispatched.

This CL updates ViewportDataGrid so that it skips to call insertBefore() if
|element| is |previousElement.nextSibling|.


[1]
https://chromium.googlesource.com/chromium/src/+/1c12127b721f833f37b81553e19b...

BUG=716583
==========

to

==========
DevTools: Fix a regression that double-clicking an item in the Network tab
doesn't open the item in a new tab.

ViewportDataGrid._update() has the following code:
    tBody.insertBefore(element, previousElement.nextSibling);
where |previousElement.nextSibling| can be |element|.

The code did nothing before [1], but we remove |element| from |tBody| and add it
to |tBody| again after [1]. Because the code is executed in 'mousedown' event
handler, 'dblclick' event for |element| wasn't dispatched.

This CL updates ViewportDataGrid so that it skips to call insertBefore() if
|element| is |previousElement.nextSibling|.


[1]
https://chromium.googlesource.com/chromium/src/+/1c12127b721f833f37b81553e19b...

BUG=716583

Review-Url: https://codereview.chromium.org/2869643002
Cr-Commit-Position: refs/heads/master@{#470833}
Committed:
https://chromium.googlesource.com/chromium/src/+/6d44fde7f95c979cd78698142389...
==========

[commit-bot: I haz the power, 2017-05-11 05:55:40]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/6d44fde7f95c979cd78698142389...

[aabadi_starconnecting.com, 2017-06-06 18:00:07]

Im very new on this, but I'm still facing this problem on my chrome version. How
can I check if this patch is already on the version I have installed on my
desktop?

Thks in advance
Rgds

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.