Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

macOS on Cirrus #4

Closed
wants to merge 9 commits into from
Closed

macOS on Cirrus #4

wants to merge 9 commits into from

Conversation

foolip
Copy link
Owner

@foolip foolip commented Oct 2, 2018

No description provided.

@foolip foolip mentioned this pull request Oct 2, 2018
Closed
@foolip foolip closed this Oct 3, 2018
@foolip foolip deleted the cirrusci-macos branch October 3, 2018 18:55
foolip pushed a commit that referenced this pull request Apr 1, 2019
@chasephillips @Hexcles
IndexedDB: Fix WPT bindings-inject-key to clean up Object.prototype
1be74ca 
chromedriver doesn't allow changing Object.prototype to add enumerable
properties, but this test requires setting some values on
Object.prototype.  When Object.prototype.a is set to:

  {b: {c: 'on proto'}}

chromedriver fails with:

    JavascriptErrorException: javascript error (500): Maximum call stack size exceeded
      (Session info: chrome=72.0.3626.121)

    Remote-end stacktrace:

    #0 0x563ff3a32a59 <unknown>
    #1 0x563ff39cb7f3 <unknown>
    #2 0x563ff38fcd7c <unknown>
    #3 0x563ff38ff78c <unknown>
    #4 0x563ff38ff5f7 <unknown>
    #5 0x563ff38ffbe7 <unknown>
    #6 0x563ff38fff1b <unknown>
    #7 0x563ff38a3f7a <unknown>
    #8 0x563ff3899bf2 <unknown>
    #9 0x563ff38a37b7 <unknown>
    #10 0x563ff3899ac3 <unknown>
    #11 0x563ff38782d2 <unknown>
    #12 0x563ff3879112 <unknown>
    #13 0x563ff39fe865 <unknown>
    #14 0x563ff39ff32b <unknown>
    #15 0x563ff39ff70c <unknown>
    #16 0x563ff39d940a <unknown>
    #17 0x563ff39ff997 <unknown>
    #18 0x563ff39e9947 <unknown>
    #19 0x563ff3a1a800 <unknown>
    #20 0x563ff3a3c8be <unknown>
    #21 0x7f3bf4545494 start_thread
    #22 0x7f3bf2d58a8f clone

    Ran 1 tests finished in 2.0 seconds.
      • 0 ran as expected. 0 tests skipped.
      • 1 tests had errors unexpectedly

Work around this problem by cleaning up the test environment so
Object.prototype no longer has the override by the time chromedriver
tries to inspect the test result.

While here, fix the other tests to use the t.add_cleanup() function
so they'll cleanup their test environment in case they exit in
some other way besides reaching t.done().

The underlying chromedriver issue is tracked upstream at
https://crbug.com/chromedriver/2555.

Bug: 934844
Change-Id: Id1b4ab2a908bfbc001e2a2d045eeec3ef01c24d9
foolip pushed a commit that referenced this pull request Jan 20, 2022
@masayuki-nakano @moz-wptsync-bot
Make HTMLEditor::HandleInsertParagraphInParagraph() call `WhiteSpac…
d84fcf4 
…eVisibilityKeeper::PrepareToSplitBlockElement()` before splitting a text node

It does the following things when caret is collapsed in a text node in a `<p>`
or `<div>` element.

1. Split the text node containing caret to insert `<br>` element
2. Insert `<br>` element after it
3. Split ancestor elements which inclusive descendants of the `<p>` or `<div>`
4. Delete the `<br>` element if unnecessary from the left paragraph

#3 and #4 are performed by `HTMLEditor::SplitParagraph()` and it calls
`WhiteSpaceVisibilityKeeper::PrepareToSplitBlockElement()` correctly before
splitting the block.  However, in the case (caret is at middle of a text node),
the text has already been split to 2 nodes because of #1.  Therefore, it fails
to handle to keep the white-space visibility.

So that I believe that the root cause of this bug is, the method does much
complicated things which are required, and doing the redundant things will
eat memory space due to undo transactions.  However, for now, I'd like to fix
this with a simple patch which just call the preparation method before splitting
the text node because I'd like to uplift this if it'd be approved (Note that
this is not a recent regression, the root cause was created by bug 92686 which
was fixed in 17 years ago:
<https://searchfox.org/mozilla-central/commit/2e66280faef73e9be218e00758d4eb738395ac83>,
but must be annoying bug for users who see this frequently).

The new WPTs are pass in Chrome.

Differential Revision: https://phabricator.services.mozilla.com/D130950

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1740416
gecko-commit: 73567f6c2bcfa078836a36760498bb11747561dd
gecko-reviewers: m_kato, smaug
foolip pushed a commit that referenced this pull request Oct 6, 2023
@mfreed7 @chromium-wpt-export-bot
Fix cloning of templates with DOM Parts, and improve testing
e03faa1 
This CL improves the testing of template cloning with Parts, testing
these four cases:

  1. Main document parsing
  2. Template (content fragment) parsing
  3. Template/fragment cloning
  4. Declarative Shadow DOM parsing and cloning

This CL fixes the behavior for #3 above, but leaves #4 broken. The
following changes in behavior are made:

1. Part::MoveToRoot() can be used to change the root(), including
   to set it to nullptr. This happens when a Node tree is removed
   from the DOM, and it contains Parts that refer to the old root.
2. IsDocumentPartRoot() is now virtual, because during a tree move,
   the root() for a Part can be made nullptr even when it's a
   ChildNodePart.
3. Part::disconnected_ is added to keep track of whether the
   Part has been disconnected, since root() can now be nullptr.
4. (This is a bug fix) When using ChildNodePart::setNextSibling(),
   the new sibling node wasn't having its Part registered with
   NodeRareData, which caused a CHECK failure when trying to
   subsequently clone that Part. This is caught in the new test
   which clones declaratively-built templates containing Parts.

Bug: 1453291
Change-Id: Ic1c1475431cf6bd658f191db78003204412ef78f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4713668
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1175782}
foolip pushed a commit that referenced this pull request May 16, 2024
@mstensho @chromium-wpt-export-bot
Get border/padding from fragment when painting background.
601b701 
Since @page border box layout objects aren't in the the layout tree, any
code that wants to walk up the tree to find the containing block will be
in for a surprise.

This would happen if percentage-based @page padding was used [1].
Recomputing padding during painting when we have already done it during
layout is rather pointless anyway. Read it out directly from the
fragment.

[1] #1 blink::LayoutBox::ContainingBlockLogicalWidthForContent()
    #2 blink::LayoutBoxModelObject::ComputedCSSPadding()
    #3 blink::LayoutBoxModelObject::PaddingTop()
    #4 blink::LayoutBoxModelObject::PaddingOutsets()
    #5 blink::BoxPainterBase::PaintFillLayer()
    #6 blink::BoxPainterBase::PaintFillLayers()
    #7 blink::BoxFragmentPainter::PaintBackground()

Bug: 40286153
Change-Id: I1e6e92c2ce1d81aab2673ec9a877eac455534102
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5526469
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1300711}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@foolip