Closed Bug 1651530 Opened 3 months ago Closed 2 months ago

min-height on table is not honored

Categories

(Core :: Layout: Tables, defect)

78 Branch
Desktop
All
defect

Tracking

()

RESOLVED FIXED
mozilla80
Tracking Status
firefox78 --- wontfix
firefox79 --- wontfix
firefox80 --- fixed

People

(Reporter: steve.patco, Assigned: emilio)

References

(Blocks 1 open bug, )

Details

(Keywords: css1, webcompat:needs-diagnosis)

Attachments

(5 files, 5 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

visit my site https://galzilla.com/index.php/blog/ in FF, the chrome and edge

Actual results:

Header image window is too small

Expected results:

Window for image should match image size

Edge and Chrome work correctly

Attached image image.png (obsolete) —

chrome to the right and FF to the left

Attached image image.png (obsolete) —

Just so you dont tell me that one is desktop and the other is android.

Attachment #9162438 - Attachment is obsolete: true
OS: Unspecified → Windows 10
Hardware: Unspecified → Desktop
Attached image image.png (obsolete) —
Attachment #9162439 - Attachment is obsolete: true

Isnt anyone going to address this issue?

Summary: header image window too small → header image window not resizing properly

Hi Steve,

I'm unabe to enter the link provided.
I've tried chrome, ff beta 79.0b7 (64-bit) and ff nightly 80.0a1 (2020-07-13) (64-bit), I'm seeing the attached error.
Please confirm if you still need us to investigate in this issue, and check on your site so I can try to replicate on my end.

Thanks for the report.

Best regards, Clara.

Flags: needinfo?(steve.patco)
Attached image cannot load page.PNG (obsolete) —

https://galzilla.com/index.php/blog/ works for me with no errors loading

Flags: needinfo?(steve.patco)

I use CC allow in my firewall. You are probably outside the zone

Hi Steve,

I was just able to access the link provided using connected to VPN USA; my apologies for the delay.
I was able to reproduce on windows 10 pro, on the following versions

Firefox Nightly version 80.0a1 (2020-07-14) (64-bit),
Beta 79.0b7
Release 78.0.2 (64-bit)

It doesn't happen in Chrome.
Setting a component for this issue in order to get the dev team involved.

Thanks for the report.
Best regards, Clara

Component: Untriaged → Graphics
Product: Firefox → Core
Status: UNCONFIRMED → NEW
Ever confirmed: true

(In reply to steve.patco from comment #0)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

visit my site https://galzilla.com/index.php/blog/ in FF, the chrome and edge

Actual results:

Header image window is too small

Expected results:

Window for image should match image size

Edge and Chrome work correctly

Be aware, CC_allow US CA MX only in CSF firewall

Component: Graphics → Layout

Potentially looks like more of a layout issue, so have moved components.

Interesting discovery. If I remove the slogan text from the header image, the image will completely disappear.

Well ALMOST disappear. No slogan will reduce image to a single line of visibility.

Is there any chance that you can attach an HTML file that reproduces the issue to the bug, using the "attach file" button?

That way europeans like me don't need to go out of our way to investigate this :-)

I can adjust the firewall to add dominant countries if you tell me your CC

I'm in Spain (ES) right now

This is about whether min-height is applied on tables. Test-case incoming.

Component: Layout → Layout: Tables
Summary: header image window not resizing properly → min-height on table is not honored
Attached file Test-case
Attachment #9162441 - Attachment is obsolete: true
Attachment #9163215 - Attachment is obsolete: true

So I noticed that the browser source code shows <!-- and --> over a good amount of code inside the Style tag. This is supposedly for the benefit of old browsers. It renders the code as comments. I am wondering if this is contributing to the problem.

I notice this can be reproduced on mac. Change this from Windows to All

Severity: -- → S3
OS: Windows 10 → All

I have a patch btw, just need to take some time to fix some tests that depend on our current behavior.

Assignee: nobody → emilio

Does this fix part or all of bug 307866?

Attached file Testcase #2 (box-sizing: content-box) (obsolete) —

Please add a box-sizing: content-box test as well, unless we already have that somewhere. I think this test renders correctly with your patch. (Chrome fails the min-height test AFAICT).

(added a couple of max-height/width tests)

Attachment #9164891 - Attachment is obsolete: true

(In reply to David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8 from comment #25)

Does this fix part or all of bug 307866?

Only part, for the table box itself, not tbody and so on I think.

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2d42454f13e1
Apply min/max-block-size to tables. r=mats
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/24682 for changes under testing/web-platform/tests
Blocks: 307866
Backout by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dc056be018fe
Backed out changeset 2d42454f13e1 for mochitest failures on test_bug1642588.html. CLOSED TREE

So the android test is an unexpected pass. But the other one is an osx-only failure in layout/generic/test/test_bug1642588.html. The bug seems pre-existing, here's the reasoning, though I cannot reproduce the failure neither on Linux nor OSX with the patch applied.

So that test is doing something like the attached test-case. After that, the test double-clicks, then backspaces. But it turns out that before my patch that backspace made the table zero-height, and after my patch it (correctly) doesn't. It probably ended up somehow messing with the selection. But you can see clearly how after double-clicking and back-spacing the selection gets completely messed up.

Kagami, you seem to have written that test, are you ok with commenting out the table test-case with a follow-up on file for it? Or should I file the bug about the broken selection and disable the test on OSX? I can't repro and I'm pretty sure the root cause is pre-existing...

Flags: needinfo?(emilio) → needinfo?(krosylight)
See Also: → 1654364

(In reply to Emilio Cobos Álvarez (:emilio) from comment #33)

Created attachment 9165178 [details]
Table contenteditable test.

So the android test is an unexpected pass. But the other one is an osx-only failure in layout/generic/test/test_bug1642588.html. The bug seems pre-existing, here's the reasoning, though I cannot reproduce the failure neither on Linux nor OSX with the patch applied.

So that test is doing something like the attached test-case. After that, the test double-clicks, then backspaces. But it turns out that before my patch that backspace made the table zero-height, and after my patch it (correctly) doesn't. It probably ended up somehow messing with the selection. But you can see clearly how after double-clicking and back-spacing the selection gets completely messed up.

Kagami, you seem to have written that test, are you ok with commenting out the table test-case with a follow-up on file for it? Or should I file the bug about the broken selection and disable the test on OSX? I can't repro and I'm pretty sure the root cause is pre-existing...

My site still does not pass on my android.

With which version of Firefox have you tried? The patch hasn't landed because it failed a test, so unless you've built firefox yourself with the patch it's not really surprising.

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e366e119efa0
Skip a subtest in test_bug1642588.html that my changes trip on automation. r=saschanaz
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2d3509d767bb
Apply min/max-block-size to tables. r=mats
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80
Upstream PR merged by moz-wptsync-bot
Flags: needinfo?(krosylight)
You need to log in before you can comment on or make changes to this bug.