Fix viewport size calculation on Firefox first load by BuildPushkar · Pull Request #1271 · kiwix/libkiwix

BuildPushkar · 2026-02-19T10:02:10Z

This fix a issue where the initial viewport size was calculated
incorrectly in Firefox when DevTools is open, resulting in fewer results.

The calculation is now based on clientWidth/clientHeight rather than
outerWidth for greater consistency.

kelson42

@BuildPushkar Thank you for your PR, without pre-judging any other feedback from my fellow reviewers, can you please first ensure a proper code formatting (spacing, indentation, lining)?

Please also update appropriate cache busting ID to fix the test. All the necessary information is in the README.md, at https://github.com/kiwix/libkiwix

BuildPushkar · 2026-02-23T07:08:18Z

@BuildPushkar Thank you for your PR, without pre-judging any other feedback from my fellow reviewers, can you please first ensure a proper code formatting (spacing, indentation, lining)?

Please also update appropriate cache busting ID to fix the test. All the necessary information is in the README.md, at https://github.com/kiwix/libkiwix

Thank you for the feedback! @kelson42
I’ll review the spacing, indentation, and overall formatting properly nd I’ll also follow the README.md regarding cache busting and update the required IDs accordingly.
I’ll push the changes soon.

kelson42 · 2026-02-23T07:19:27Z

@BuildPushkar thx
@rgaudin Does it solve the bug for you?

veloman-yunkan · 2026-02-23T09:22:25Z

static/skin/index.js

+  let zoom = 1;
+
+  if (window.innerWidth) {
+    zoom = Math.floor((width / window.innerWidth) * 100) || 1;
+  }


What is the meaning of the zoom variable? The presence of * 100 in the calculation implies that it is measured in % which is in contradiction with the default value of 1.

Thanks for pointing this out.
I’ve updated the logic to use a zoom ratio instead of a percentage and removed the *100 factor, so it now reflects the actual layout to viewport ratio.

veloman-yunkan · 2026-02-23T09:26:01Z

static/skin/index.js

+  const rows = Math.floor(height / (3 * zoom) + 1);
+  const cols = Math.floor(width / (2.5 * zoom) + 1);


I don't understand the logic of this calculation. IMHO, the formula for the number of rows/columns should involve the height/width of the items being laid out in a grid (and maybe vertical/horizontal spacing between them), but there are some magic numbers 3 and 2.5 which bear no meaning to me.

I see that it is carried over from the previous implementation, but I think that we should strive to bring more clarity into code that we have to modify for any reason.

Good point, thanks.
These values were inherited from the previous implementation, but I’ve now extracted them into named constants with comments to make their purpose clearer. Let me know if you’d prefer a different approach.

Introduction of CARD_HEIGHT_FACTOR/CARD_WIDTH_FACTOR doesn't help much if at all. I'd rather want to see a formula that makes geometrical sense (something like height/(cardHeight + verticalSpacing).

Thank you, I get your point.
The constants were mainly to keep the existing behavi0r for now. i agree that a formula based on actual card height and spacing would make more sense. I’ll try to rework this part using a geometry based approach

Please don't take too much pain pursuing this improvement. If it doesn't come easy, just clean-up this PR so that the addressed bug is fixed with minimal changes to the existing code and the CI is green (unit tests pass).

Got it, thank you
I’ll keep this PR focused on fixing the Firefox viewport issue with minimal changes and make sure everything is clean.

rgaudin · 2026-02-23T12:16:06Z

@BuildPushkar thx @rgaudin Does it solve the bug for you?

Yes it does 👍

veloman-yunkan · 2026-02-23T12:30:09Z

static/skin/index.js

-    function viewPortToCount(){
-        const zoom = Math.floor((( window.outerWidth - 10 ) / window.innerWidth) * 100);
-        return Math.floor((window.innerHeight/(3*zoom) + 1)*(window.innerWidth/(2.5*zoom) + 1));
+    function viewPortToCount() {
+        // Use stable viewport size instead of outerWidth (Firefox devtools issue)
+        const width = 
+            document.documentElement.clientWidth || window.innerWidth;
+
+        const height =
+            document.documentElement.clientHeight || window.innerHeight;
+
+        // Zoom ratio btw layout width and viewport width
+        let zoomRatio = 1;
+
+        if (window.innerWidth) {
+            zoomRatio = width / window.innerWidth || 1;
+        }
+
+        // Estimated card size (in viewport units)
+        const CARD_HEIGHT_FACTOR = 3;
+        const CARD_WIDTH_FACTOR = 2.5;
+
+        const rows = Math.floor(
+            height / (CARD_HEIGHT_FACTOR * zoomRatio) + 1
+        );
+
+        const cols = Math.floor(
+            width / (CARD_WIDTH_FACTOR * zoomRatio) + 1
+        );


I think this won't work. Before this change zoom used to be percentage, but the formulas for rows and cols have been actually preserved in spite of the fact that zoomRatio is now being used instead.

Thanks for pointing this out, I see what you mean.
The goal was to switch to a ratio while keeping the existing behavior stable but I agree this may not fully preserve the original scaling.
I’ll revisit the calculation and see how we can better align it with the new zoomRatio semantics.

BuildPushkar · 2026-02-28T09:06:46Z

@kelson42 are there any updates regarding this PR?

i noticed the “changes requested” badge is still present.
Pls let me know if there’s anything else i should address.
Thanks!

kelson42 · 2026-02-28T09:11:44Z

@BuildPushkar I was kind of waiting to you ;)

BuildPushkar · 2026-02-28T09:18:15Z

@BuildPushkar I was kind of waiting to you ;)

Sorry for the waiting. My lap stopped working, so I had to borrow a friend’s lap to continue my work.

BuildPushkar · 2026-02-28T09:31:11Z

@kelson42 whenever you free, let me know should i make changes? i’ll be happy to address them.
thank you :)

kelson42 · 2026-02-28T09:36:14Z

@kelson42 whenever you free, let me know should i make changes? i’ll be happy to address them.
thank you!

I'm a kind of facilitator here.

Hard core reviewer is @veloman-yunkan.

But please remove the main merges. There are kind of taboo by us because they make the revision log really dirty.

Once the PR review over, we will do a rebase on main. Can you please do that?

kelson42 · 2026-03-01T00:22:24Z

@veloman-yunkan AFAIK we are read here for a new review pass

veloman-yunkan · 2026-03-01T08:37:42Z

static/skin/index.js

-    function viewPortToCount(){
-        const zoom = Math.floor((( window.outerWidth - 10 ) / window.innerWidth) * 100);
-        return Math.floor((window.innerHeight/(3*zoom) + 1)*(window.innerWidth/(2.5*zoom) + 1));
+    function viewPortToCount() {
+        // Use stable viewport size instead of outerWidth (Firefox devtools issue)
+        const width = 
+            document.documentElement.clientWidth || window.innerWidth;
+
+        const height =
+            document.documentElement.clientHeight || window.innerHeight;
+
+        // Zoom ratio btw layout width and viewport width
+        let zoomRatio = 1;
+
+        if (window.innerWidth) {
+            zoomRatio = width / window.innerWidth || 1;
+        }
+
+        // Estimated card size (in viewport units)
+        const CARD_HEIGHT_FACTOR = 3;
+        const CARD_WIDTH_FACTOR = 2.5;
+
+        const rows = Math.floor(
+            height / (CARD_HEIGHT_FACTOR * zoomRatio) + 1
+        );
+
+        const cols = Math.floor(
+            width / (CARD_WIDTH_FACTOR * zoomRatio) + 1
+        );


Has this code been tested? The concern raised during the previous has not been addressed, and therefore it looks like the functionality should be utterly broken unless the return value of viewPortToCount() (which is now ~10000 larger than before the change) doesn't matter at all (which would mean a different kind of defect of the implementation).

veloman-yunkan · 2026-03-01T08:42:39Z

Actually, it looks like no changes other than bringing updates from the main branch into this PR have been done since last review. I wonder why another review has been requested then.

BuildPushkar · 2026-03-01T09:45:17Z

Apologies the branch update was accidental and did not include functional changes.

I’m currently re-evaluating the viewport calculation logic based on the review comments and will push an actual fix shortly.

@kelson42 whenever you free, let me know should i make changes? i’ll be happy to address them.
thank you!

I'm a kind of facilitator here.

Hard core reviewer is @veloman-yunkan.

But please remove the main merges. There are kind of taboo by us because they make the revision log really dirty.

Once the PR review over, we will do a rebase on main. Can you please do that?

yes, i’ll clean up the branch history and remove the merge commits. i’ll rebase properly once the review changes are addressed.

BuildPushkar · 2026-03-01T10:05:38Z

Actually, it looks like no changes other than bringing updates from the main branch into this PR have been done since last review. I wonder why another review has been requested then.

i mistakenly updated the branch without functional changes that wasn’t intentional and re-requested the review for the understading where im doing wrong

BuildPushkar · 2026-03-01T10:06:12Z

Actually, it looks like no changes other than bringing updates from the main branch into this PR have been done since last review. I wonder why another review has been requested then.

i mistakenly updated the branch without functional changes(that wasn’t intentional) and re-requested the review for the understading where im doing wrong

BuildPushkar · 2026-03-01T11:02:52Z

i've reworked the function to derive rows and columns from actual card dimensions and margins instead of using zoombased scaling.
The calculation now follows:
viewport / (card size + spacing)
Sharing this implementation before pushing, does this align with what you had in mind? @veloman-yunkan

veloman-yunkan · 2026-03-02T10:17:25Z

@BuildPushkar That code makes sense, but please test it for various window sizes and zoom ratios.

BuildPushkar · 2026-03-03T10:41:54Z

@BuildPushkar That code makes sense, but please test it for various window sizes and zoom ratios.

yeah sure

BuildPushkar · 2026-03-13T08:26:13Z

sorry for the delay, my life is giving me a lot of trouble :)

i'm having trouble reproducing the exact testing setup locally.
@veloman-yunkan could you clarify the recommended way to test the homepage result calculation locally?
i thought i could figure it out by myself, however I'm still a beginner.

Thanks!

veloman-yunkan · 2026-03-13T10:21:15Z

@BuildPushkar I am not sure that I understand the problems that you are having and the advice that you are seeking.

The simple flow is to build kiwix-tools (which is a separate project depending on libkiwix), run kiwix-serve and access its front-end via a web browser.

If you have to do changes in the JS code during testing there is a more efficient flow eliminating the need to build kiwix-tools every time you update the sources in libkiwix (see #779).

Fix viewport size calculation on Firefox first load

351e4e1

kelson42 requested review from kelson42, rgaudin and veloman-yunkan February 22, 2026 16:57

kelson42 requested changes Feb 22, 2026

View reviewed changes

veloman-yunkan reviewed Feb 23, 2026

View reviewed changes

BuildPushkar added 2 commits February 23, 2026 15:30

Fix formatting and update cache busting IDs"

fd155b4

Fix code formatting and update cache busting IDs for statics files

cf3ca17

veloman-yunkan reviewed Feb 23, 2026

View reviewed changes

BuildPushkar added 2 commits February 24, 2026 13:35

Merge branch 'main' into fix-homepage-results

b3c356c

Merge branch 'main' into fix-homepage-results

851d4a7

kelson42 requested a review from veloman-yunkan March 1, 2026 00:20

veloman-yunkan requested changes Mar 1, 2026

View reviewed changes

		const rows = Math.floor(height / (3 * zoom) + 1);
		const cols = Math.floor(width / (2.5 * zoom) + 1);

Uh oh!

Conversation

BuildPushkar commented Feb 19, 2026

Uh oh!

kelson42 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BuildPushkar commented Feb 23, 2026

Uh oh!

kelson42 commented Feb 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

veloman-yunkan Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rgaudin commented Feb 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BuildPushkar commented Feb 28, 2026

Uh oh!

kelson42 commented Feb 28, 2026

Uh oh!

BuildPushkar commented Feb 28, 2026

Uh oh!

BuildPushkar commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kelson42 commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kelson42 commented Mar 1, 2026

Uh oh!

veloman-yunkan Mar 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

veloman-yunkan commented Mar 1, 2026

Uh oh!

BuildPushkar commented Mar 1, 2026

Uh oh!

BuildPushkar commented Mar 1, 2026

Uh oh!

BuildPushkar commented Mar 1, 2026

Uh oh!

BuildPushkar commented Mar 1, 2026

Uh oh!

veloman-yunkan commented Mar 2, 2026

Uh oh!

BuildPushkar commented Mar 3, 2026

Uh oh!

BuildPushkar commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

veloman-yunkan commented Mar 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

kelson42 left a comment •

edited

Loading

veloman-yunkan Feb 23, 2026 •

edited

Loading

BuildPushkar commented Feb 28, 2026 •

edited

Loading

kelson42 commented Feb 28, 2026 •

edited

Loading

veloman-yunkan Mar 1, 2026 •

edited

Loading

BuildPushkar commented Mar 13, 2026 •

edited

Loading