Align position of unstyled list items' bullet point with IE firefox

Align position of unstyled list items' bullet point with IE firefox

BUG=590094
TEST=Load https://jsfiddle.net/2qgcuofg/2/
R=skobes@chromium.org,nainar@chromium.org

[xing.xu, 2016-09-25 13:41:54]

Description was changed from

==========
Align position of unstyled list items' bullet point with IE firefox

BUG=590094
TEST=Load https://jsfiddle.net/2qgcuofg/2/
R=skobes@chromium.org,nainar@chromium.org
==========

to

==========
Align position of unstyled list items' bullet point with IE firefox

BUG=590094
TEST=Load https://jsfiddle.net/2qgcuofg/2/
R=skobes@chromium.org,nainar@chromium.org
==========

[xing.xu, 2016-09-25 13:43:18]

Hi, please help to review.

BTW, in the previous implementation, it comments "If we are not in a list, tell
the layoutObject so it can position us inside. We don't want to change our style
to say "inside" since that would affect nested nodes." Do you have any
background for this?

[nainar, 2016-09-25 16:55:22]

On 2016/09/25 at 13:43:18, xing.xu wrote:
> Hi, please help to review.
> 
> BTW, in the previous implementation, it comments "If we are not in a list,
tell the layoutObject so it can position us inside. We don't want to change our
style to say "inside" since that would affect nested nodes." Do you have any
background for this?

Hi, 

Just checking have you run the LayoutTests on this change? Also I am scheduling
it for CQ dry run. So I can get an idea of how this fares on the bots. 

Also any change in code base should also have a LayoutTest for it so please add
one here. 

The change seems ok - will l*tm once the due diligence is done. I'm not well
versed in DOM stuff but will look into this and get back to you.

[nainar, 2016-09-25 16:55:26]

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

[commit-bot: I haz the power, 2016-09-25 16:55:36]

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, 2016-09-25 17:55:15]

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

[commit-bot: I haz the power, 2016-09-25 17:55:16]

Dry run: 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_...)

[nainar, 2016-09-26 03:05:44]

On 2016/09/25 at 16:55:22, nainar wrote:
> On 2016/09/25 at 13:43:18, xing.xu wrote:
> > Hi, please help to review.
> > 
> > BTW, in the previous implementation, it comments "If we are not in a list,
tell the layoutObject so it can position us inside. We don't want to change our
style to say "inside" since that would affect nested nodes." Do you have any
background for this?
> 
> Hi, 
> 
> Just checking have you run the LayoutTests on this change? Also I am
scheduling it for CQ dry run. So I can get an idea of how this fares on the
bots. 
> 
> Also any change in code base should also have a LayoutTest for it so please
add one here. 
> 
> The change seems ok - will l*tm once the due diligence is done. I'm not well
versed in DOM stuff but will look into this and get back to you.

Yup definitely looks like you need to look into the failing LayoutTests...

[xing.xu, 2016-09-28 14:30:03]

Thanks. Will look into these tests. BTW, it seems some regression cases are
platform dependent, does it possible change these to RefTests as
https://trac.webkit.org/wiki/Writing%20Reftests?

[nainar, 2016-09-29 04:09:21]

Yup feel free to changes those tests. You can even chnage them to use the
testharness API in LayoutTests/resources.

[nainar, 2016-10-10 07:12:20]

The change lgtm but please do fix up the tests, OWNERS lgtm. You can't CQ until
that anyway.

[xing.xu, 2016-10-10 07:40:12]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 07:40:21]

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, 2016-10-10 07:42:10]

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

[commit-bot: I haz the power, 2016-10-10 07:42:11]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[xing.xu, 2016-10-10 08:40:12]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 08:40:21]

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, 2016-10-10 09:50:09]

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

[commit-bot: I haz the power, 2016-10-10 09:50:10]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[xing.xu, 2016-10-10 14:17:35]

Hi, nainar@, is there any plan to refactor all these platform dependent tests to
reftests or jstests?

[nainar, 2016-10-10 14:20:24]

Hi xing.xu, 

Unfortunately I don't think there are any short term plans to undergo that
refactor. I would talk to dpranke@ or ojan@ in this regard.

[xing.xu, 2016-10-11 00:40:53]

OK, thanks.

Summary all failed webkit_tests here:
win_chromium_rel_ng
unexpected_failures:
fast/text/font-weight.html
fast/text/textIteratorNilRenderer.html
paint/invalidation/scrolled-iframe-scrollbar-change.html
fast/text/font-stretch-variant.html
paint/invalidation/do-not-paint-below-image-baseline.html
fast/text/sub-pixel/text-scaling-pixel.html
fast/text/international/bidi-listbox.html
fast/text/international/bidi-listbox-atsui.html
fast/text/international/lang-glyph-cache-separation.html
fast/text/font-stretch.html
fast/text/drawBidiText.html
paint/invalidation/transform-rotate-and-remove.html
fast/text/font-features/caps-native-synthesis.html

linux_chromium_rel_ng   
unexpected_failures:
paint/invalidation/transform-rotate-and-remove.html
paint/invalidation/scrolled-iframe-scrollbar-change.html
paint/invalidation/do-not-paint-below-image-baseline.html

mac_chromium_rel_ng
unexpected_failures:
paint/invalidation/transform-rotate-and-remove.html
paint/invalidation/scrolled-iframe-scrollbar-change.html
paint/invalidation/do-not-paint-below-image-baseline.html