Skip to content

/ servo

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.

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

Intermittent failure in /_mozilla/css/border_collapse_simple_a.html #8120

Closed
Manishearth opened this issue Oct 21, 2015 · 4 comments
Closed

Intermittent failure in /_mozilla/css/border_collapse_simple_a.html #8120

Manishearth opened this issue Oct 21, 2015 · 4 comments
Assignees
@mbrubeck
Labels
A-layout/table I-intermittent

Comments

@Manishearth
Copy link
Member

@Manishearth Manishearth commented Oct 21, 2015

#8119 (comment)

/_mozilla/css/border_collapse_simple_a.html 77802e92a2da4dd031b24c4be665f39f9062e055
/_mozilla/css/border_collapse_simple_ref.html 95a8130381addac71d77940ba243397b634e1a43
@Manishearth Manishearth mentioned this issue Oct 21, 2015
Merged
@jdm
Copy link
Member

@jdm jdm commented Oct 21, 2015

There's a one pixel difference in the output. Looks like a layout problem to me.
screen shot 2015-10-21 at 9 39 33 am
@jdm jdm added the A-layout/table label Oct 21, 2015
@Ms2ger Ms2ger mentioned this issue Oct 22, 2015
Merged
@Manishearth Manishearth mentioned this issue Oct 27, 2015
Merged
@jdm jdm changed the title Intermittent in border_collapse_simple_a.html Intermittent failure in border_collapse_simple_a.html Jan 3, 2016
@jdm jdm mentioned this issue Jan 3, 2016
Merged
@KiChjang KiChjang mentioned this issue Jan 8, 2016
Merged
This was referenced Jan 30, 2016
Merged
Merged
Merged
Merged
Merged
@wafflespeanut wafflespeanut mentioned this issue Feb 6, 2016
Merged
@KiChjang KiChjang mentioned this issue Feb 8, 2016
Merged
This was referenced Feb 13, 2016
Merged
Merged
@emilio emilio mentioned this issue Feb 13, 2016
Merged
@jdm jdm mentioned this issue Feb 14, 2016
Merged
@nox nox mentioned this issue Feb 16, 2016
Merged
This was referenced Feb 16, 2016
Merged
Merged
@jdm jdm mentioned this issue Feb 17, 2016
Merged
@KiChjang KiChjang mentioned this issue Feb 18, 2016
Merged
@nox nox mentioned this issue Feb 23, 2016
Merged
@jdm jdm mentioned this issue Feb 24, 2016
Merged
@wafflespeanut wafflespeanut mentioned this issue Feb 25, 2016
Merged
@frewsxcv frewsxcv mentioned this issue Feb 26, 2016
Merged
@KiChjang KiChjang mentioned this issue Feb 26, 2016
Merged
@nox nox mentioned this issue Feb 27, 2016
Merged
@nox
Copy link
Member

@nox nox commented Feb 27, 2016

Cc @mbrubeck

This is again an incremental layout bug.

Layout is first done with only the first and second rows, resulting in a bottom border of 4px. The other rows are then parsed and laid out, but the bottom border of the second row isn't updated to 2px.
This was referenced Feb 27, 2016
Merged
Merged
@frewsxcv frewsxcv mentioned this issue Feb 28, 2016
Merged
@KiChjang KiChjang mentioned this issue Feb 28, 2016
Merged
@nox nox mentioned this issue Feb 29, 2016
Merged
@nox nox changed the title Intermittent failure in border_collapse_simple_a.html Intermittent failure in /_mozilla/css/border_collapse_simple_a.html Feb 29, 2016
nox added a commit to nox/servo that referenced this issue Feb 29, 2016
@nox
Vindictively disable all intermittent tests that failed servo#9787
29d97b6 
This disables the tests mentioned in servo#8120, servo#9014, servo#9092, servo#9148, servo#9205 and servo#9803.
@nox nox mentioned this issue Feb 29, 2016
Merged
bors-servo added a commit that referenced this issue Feb 29, 2016
@bors-servo
Auto merge of #9804 - nox:vengeful-disabling, r=KiChjang
591bc8d 
Vindictively disable all intermittent tests that failed #9787

This disables the tests mentioned in #8120, #9014, #9092, #9148, #9205 and #9803.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9804)
<!-- Reviewable:end -->
nox added a commit to nox/servo that referenced this issue Feb 29, 2016
@nox
Vindictively disable all intermittent tests that failed servo#9787
7aa2b6e 
This disables the tests mentioned in servo#8120, servo#9014, servo#9092, servo#9148, servo#9205 and servo#9803.
nox added a commit to nox/servo that referenced this issue Feb 29, 2016
@nox
Vindictively disable all intermittent tests that failed servo#9787
863a5c5 
This disables the tests mentioned in servo#8120, servo#9014, servo#9092, servo#9148, servo#9205 and servo#9803.
bors-servo added a commit that referenced this issue Feb 29, 2016
@bors-servo
Auto merge of #9804 - nox:vengeful-disabling, r=KiChjang
8c7f42f 
Vindictively disable all intermittent tests that failed #9787

This disables the tests mentioned in #8120, #9014, #9092, #9148, #9205 and #9803.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9804)
<!-- Reviewable:end -->
nox added a commit to nox/servo that referenced this issue Feb 29, 2016
@nox
Vindictively disable all intermittent tests that failed servo#9787
ea1c169 
This disables the tests mentioned in servo#8120, servo#9014, servo#9033, servo#9092, servo#9148, servo#9205, servo#9772 and servo#9803.
bors-servo added a commit that referenced this issue Feb 29, 2016
@bors-servo
Auto merge of #9804 - nox:vengeful-disabling, r=KiChjang
Loading status checks…
2ba1709 
Vindictively disable all intermittent tests that failed #9787

This disables the tests mentioned in #8120, #9014, #9033, #9092, #9148, #9205, #9772 and #9803.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9804)
<!-- Reviewable:end -->
@mbrubeck mbrubeck self-assigned this Feb 29, 2016
mbrubeck added a commit to mbrubeck/servo that referenced this issue Feb 29, 2016
@mbrubeck
Fix border collapsing at the end of a table-row-group
394e525 
Fixes servo#8120
mbrubeck added a commit to mbrubeck/servo that referenced this issue Feb 29, 2016
@mbrubeck
Fix border collapsing at the end of a table-row-group
1d9bb65 
This fixes the border-end calculation for a table row that is the last in its
rowgroup but not the last in its table.  Fixes servo#8120
mbrubeck added a commit to mbrubeck/servo that referenced this issue Mar 1, 2016
@mbrubeck
Fix border collapsing at the end of a table-row-group
fdca337 
This fixes the border-end calculation for a table row that is the last in its
rowgroup but not the last in its table.  Fixes servo#8120
@mbrubeck
Copy link
Contributor

@mbrubeck mbrubeck commented Mar 1, 2016

Part of the issue is that inserting a row into a table dynamically can lead to a flow tree like this:

    <table>
      <tbody>
        <tr><td></td></tr>
      </tbody>
      <tr><td></td></tr>
    </table>

The border-collapse code in Servo has a bug that prevents it from traversing this flow tree correctly. Fixing that bug fixes the border width. I haven't submitted that fix yet because some other bug that's causing the position of the border to be off by 1px in my test case. Investigating that now.
@nox
Copy link
Member

@nox nox commented Mar 1, 2016

@mbrubeck Oh nice!
mbrubeck added a commit to mbrubeck/servo that referenced this issue Mar 1, 2016
@mbrubeck
Fix border collapsing at the end of a table-row-group
6c684a5 
This fixes the border-end calculation for table rows whose borders are
collapsed with rows in different rowgroups.  The border collapsing code now
uses an iterator that yields all the rows as a flat sequence, regardless of
how they are grouped in rowgroups.  It gets rid of
`TableRowGroupFlow::preliminary_collapsed_borders` which was never correct.
(It was read but never written.)

This may fix servo#8120 but I'm not 100% certain. (I haven't managed to reproduce
the intermittent failure locally, and my reduced test case still fails but in
a different way.)
@mbrubeck mbrubeck mentioned this issue Mar 1, 2016
Merged
bors-servo added a commit that referenced this issue Mar 1, 2016
@bors-servo
Auto merge of #9826 - mbrubeck:table-row-iter, r=pcwalton
Loading status checks…
1d6aece 
Table border-collapse fixes

Two related fixes for border-collapse:

* Fix border collapsing across table-row-group flows

  This fixes the border-end calculation for table rows whose borders are collapsed with rows in different rowgroups.  The border collapsing code now uses an iterator that yields all the rows as a flat sequence, regardless of how they are grouped in rowgroups.  It gets rid of `TableRowGroupFlow::preliminary_collapsed_borders` which was never correct.  (It was read but never written.)

  This may fix #8120 but I'm not 100% certain. (I haven't managed to reproduce the intermittent failure locally, and my reduced test case still fails but in a different way.)

* Fix confusing `push_or_mutate` API

  This fixes a bug when recalculating border collapsing for an existing table row. The bug was caused by using `push_or_mutate` which has no effect if there is already a value at the specified index.

  The fix switches incorrect `push_or_mutate` calls to use `push_or_set` instead. It also renames `push_or_mutate` to `get_mut_or_push` which I think is a less-confusing name for this method.

r? @pcwalton

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9826)
<!-- Reviewable:end -->
@nox nox mentioned this issue May 20, 2016
Merged
3 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mbrubeck mbrubeck

Labels
A-layout/table I-intermittent
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

Table border-collapse fixes
4 participants
@mbrubeck @jdm @nox @Manishearth
You can’t perform that action at this time.