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

CSS Layout Fighting in Options Page #708

Closed
Tomalak opened this issue Jul 30, 2021 · 17 comments
Closed

CSS Layout Fighting in Options Page #708

Tomalak opened this issue Jul 30, 2021 · 17 comments

Comments

@Tomalak
Copy link

Tomalak commented Jul 30, 2021

The options page currently does this (Tenten 1.2.0, Firefox 90.0.2 64-bit, Windows 10)

tenten
@birtles
Copy link
Member

birtles commented Jul 30, 2021

Interesting. It works for me so I wonder what's different. I've tried with Firefox 92 and Firefox 91 on Windows 10 and I can't reproduce that. I'll see if I can try with Firefox 90.

@birtles
Copy link
Member

birtles commented Jul 30, 2021

Hmm, I can't reproduce it in Firefox 90 either. Does it do it endlessly? Or only while downloading updates? Does it stop if you change the settings in any way?

The only other factors I can think of are:

  • Different system fonts (I'm on Japanese Windows)
  • Different display density/resolution (I'm on a HiDPI monitor of some sort with 150% magnification but even if I turn it down to 100% I can't reproduce)

@Tomalak
Copy link
Author

Tomalak commented Jul 30, 2021 •
edited
Loading

Yes it never stops flickering, but I still can change the settings.

Might be resolution related? I'm on 2560 x 1440 px with 175% scaling.

@Tomalak
Copy link
Author

Tomalak commented Jul 30, 2021

...yep, that seems to be scaling-related. Going to 200% scaling makes it stop.

@birtles
Copy link
Member

birtles commented Jul 30, 2021

Hmm, that's interesting. I tried that same resolution and magnification using Firefox 90 and I still can't reproduce it. Maybe fonts are related too.

Ultimately it's probably a Firefox bug since I don't think that options page is doing anything fancy. So if static content is producing layout thrashing it suggests an instability in Firefox layout code. Nevertheless we still need to fix it -- but it's a bit hard to know how without being able to reproduce it 🤔

@birtles
Copy link
Member

birtles commented Jul 30, 2021

If anyone can consistently reproduce this, perhaps the most helpful thing would be:

  1. Go to about:debugging
  2. Choose "This Firefox"
  3. Find "10ten Japanese Reader (Rikaichamp)"
  4. Choose "Inspect"
  5. In the little frames icon in the top right open the drop-down and choose /options.html
  6. Move the tab to a new window (e.g. drag the tab title downwards)
  7. Use the inspect tool (the icon with the box and cursor on the far left of the inspector toolbar) to select the options panel
  8. Progressively select elements and delete them until the flickering stops

If you can determine which element is causing the flickering then that will help us know what we need to fix.

@Tomalak
Copy link
Author

Tomalak commented Jul 31, 2021

I notice that it constantly triggers "overflow" at the panel-section-db-summary div, and in consequence, "scroll" events at the <html> element:

image

It stops when I either

  • delete the entire panel-section-db-summary div, or
  • disable the CSS properties highlighted below. If I turn any of them back on, the problem reappears.

image

It also stops when I remove any of the <fieldset> elements that contain icon selection, popup style and tab position.

Of course this could be a red herring because triggering a reflow of the layout could make it go away naturally.

It also stops when I delete the key-grid, or change it from grid-template-columns: minmax(12em, auto) 1fr; to a fixed value like grid-template-columns: minmax(12em, 5em) 1fr; - it could also be a bug in Firefox's grid layout code.

@Tomalak
Copy link
Author

Tomalak commented Jul 31, 2021 •
edited
Loading

The smallest-possible change I found to stabilize the layout is going from

.panel-section-db-summary > :not(:first-child) {
    margin-top: 20px;
}

to

.panel-section-db-summary > :not(:first-child) {
    margin-top: 14px;
}

but then again, this very possibly only circumvents the rounding issue in some internal layout code in Firefox from occurring, without addressing the root issue.

Another thing that works is disabling flex-wrap: wrap; in .panel-section-db-summary > .db-summary-status.

It really looks like it is margin- and/or flexbox-related, and as if too many things of equal priority on the page are relative to each other.

@birtles
Copy link
Member

birtles commented Aug 2, 2021 •
edited
Loading

Yeah, this is really odd. I wonder if it's because Firefox manually sets the page height of the options page:

https://searchfox.org/mozilla-central/rev/4b88e0b8cca115009e82fdd65e5bf5812ff99128/toolkit/mozapps/extensions/content/aboutaddons.js#2446

Changes to the DOM of the page trigger that to be updated:

https://searchfox.org/mozilla-central/rev/4b88e0b8cca115009e82fdd65e5bf5812ff99128/toolkit/components/extensions/ext-browser-content.js#310

I wonder if we're running into trouble because of some of the responsive elements in the page.

e.g. maybe we have something like the following situation:

  1. Do layout and overflow height producing scrollbars
  2. Extension:BrowserResized event is dispatched and the height of the container is updated so now scrollbars are no longer needed
  3. As a result we reflow and end up with a different (shorter) height
  4. That triggers a new Extension:BrowserResized and we update the height again taking us back to (1)

That might fit with the observation that converting the grid properties to something less responsive works around the problem.

@birtles
Copy link
Member

birtles commented Aug 2, 2021

Alright, I think I might know a fix. I think if we put overflow: hidden on the root <html> element we might avoid the scrollbars appearing and re-appearing and causing the instability. We'll need to only do that for Firefox, however, since the other browsers probably need the scrollbars.

@birtles birtles closed this as completed in e8073d4 Aug 2, 2021
@Tomalak
Copy link
Author

Tomalak commented Aug 3, 2021

Sounds like a pragmatic fix to me

We'll need to only do that for Firefox, however, since the other browsers probably need the scrollbars.

Don't forget to test on mobile Firefox :)

@birtles
Copy link
Member

birtles commented Aug 3, 2021

Don't forget to test on mobile Firefox :)

I don't suppose you can help?

I've never even tried running it on Firefox for Android.

@Tomalak
Copy link
Author

Tomalak commented Aug 3, 2021

Sure, I can try to help with that!

Disclaimer: I'm using mobile Firefox as my main browser, and I currently don't have 10-ten installed, but I can give it a go and if you have any specifics I should test, let me know.

@Tomalak
Copy link
Author

Tomalak commented Aug 3, 2021

...and I've hit the first roadblock - apparently 10-ten is not marked as compatible with Firefox for Android at the moment.

I suppose that's only a matter of adjusting the manifest?

@birtles
Copy link
Member

birtles commented Aug 5, 2021

...and I've hit the first roadblock - apparently 10-ten is not marked as compatible with Firefox for Android at the moment.

I suppose that's only a matter of adjusting the manifest?

Sorry for the delay here. Firefox for Android only allows recommended extensions at the moment.

Information here: Extensions in Firefox for Android Update

I think the criteria for being a Recommended Extension includes being "relevant to a general, international audience" (https://support.mozilla.org/en-US/kb/recommended-extensions-program) for which I don't think any foreign language tool will ever qualify.

That said, it can be installed on Firefox Nightly, see: Expanded extension support in Firefox for Android Nightly. I know some people are doing that although I've personally never tried.

@Tomalak
Copy link
Author

Tomalak commented Aug 6, 2021

Then let's disregard my cautionary remark. I was under the impression that Firefox for Android supported running all extensions, but I've never put that to the test. Thanks for the heads-up.

@birtles
Copy link
Member

birtles commented Aug 7, 2021

No problem. Some day I'd like to tweak Firefox for Android support so that if/when Mozilla do allow more extensions to run there we'll be ready to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Tomalak @birtles