Bug - Fixed extra intraneous scrollbar

fredg1

Member
This has been a thing for a LONG time (definition of "long" may vary, considering I've only been here for 3 years now), and I'm surprised I didn't manage to find any report of this so far.


Mafia seems to be adding a 2nd scrollbar to fight.php result pages seemingly at random.
BDHeOblhPE_part_1.png
BDHeOblhPE_part_2.png

This intraneous scrollbar spans the entire height of the pre-existing page, even after considering the content that mafia may add to it (such as the "Found this fight" text), meaning that scrolling this scroll-bar achieves absolutely nothing beneficial. The amount it lets you scroll down also seems to correspond to the amount of content added to the bottom of the page by mafia; maybe what this is is mafia doing two things at once to fix the same "issue" (increasing the size of the preexisting page, AND adding the scroll-bar)?

t8aumQIjgt.png

This would simply be a "quirky, peculiar little interaction" if it wasn't for this fact: the intraneous scroll-bar has priority. This means that any attempt to scroll the page will first scroll "this" scroll-bar, which won't go far, and which will then prevent you from scrolling any further for a good 1-2 seconds if you don't move the mouse.

I grabbed the frame's source, in case it helps: https://pastebin.com/BLiP5PdM
 

Veracity

Developer
Staff member
I'll let somebody who uses (has ever used) the CAB investigate this.
I have literally never seen this using KoLmafia's Stationary Buttons.
 

fredg1

Member
Ok, so with the help of Katarn and Zdrvst, we've been digging, and found where this comes from: turns out this is unrelated to the CAB.

Turns out, this actually comes from the "You acquire an item:" sections. More precisely, acquiring any type of equipment.


First, little reminder: if you acquire an equipment item, mafia will add an [equip] link right of it. (If it's a weapon, there'll be [equip] and [offhand])
Well, when hovering over one of these links, mafia will actually show a popup message showing (...or attempting to) how your character will change once that item is equipped.
This/these popups' size, being pre-determined on page load, then hidden, are what pushes the page downwards.

THEN, the fact that the page is set to style="overflow: auto;" is what causes this extra length to be converted into an additional scroll-bar, instead of merging it with the previous one!

firefox_fabaFe7Xje.png
 

zarqon

Well-known member
If I recall correctly, the overflow issue is still slightly related to the CAB. On fight pages, without the CAB the content is all in the <body> tags, like most web pages in existence. With the CAB enabled, the content is inexplicably all in a div that isn't even in the body and has to have its height set by CSS.

This is based on my memory from years ago when I was making BatMan RE compatible with the CAB, which I recall being a bit of a headache due to this issue. This may no longer be correct, but I'm sharing just in case it's helpful in solving the issue -- and I do suspect the issue will not occur for non-CAB users.
 

fredg1

Member
Ah, yes, I guess I mixed things up when we discovered what was the cause of the added white space at the end of pages.

What mixed things up was seeing that it also happened in very small "you acquire an item" sections:
unknown.png
20200910_kol_mafia_scrollbar.png

So let me get things clear:
What causes the additional white space at the end of pages is that hidden popup(s), which, in itself, may be seen as a bug???

If you are using the CAB while getting this extra white space, then you get the bug mentioned in this thread, which is an additional scroll-bar instead of extending the current one.
 

fredg1

Member
Sorry, I was wrong; it's not just the [equip] links, potions' [use] link do that, too, and I'm surely forgetting other things...firefox_azHJxe6Jpn.png
 
Last edited:

fredg1

Member
I seem to have found a solution.

The problem gravitates around the fact mafia toggles the table's "visibility" when hovering over it.
firefox_QwGSMfsCFc.pngfirefox_CNfubnC5bW.png

This problem is "solved" (leaving space for doubt, in case it secretly breaks something) by having it toggle it's "display":
firefox_WDleGyNeaE.pngfirefox_GmiDW4U8Jl.png
Done in this patch: View attachment UseLinkDecorator.java.patch

This only triggers the intraneous scroll-bar to appear when you hover over the link. Already an improvement.
The only issue here is that the scrollbar "bats" the link a little further to the left, so there's some glitching happening if you try to hover on the right side of the link (it frantically alternates between displayed and not displayed ).
 

fredg1

Member
The intraneous scroll-bar itself is directly caused by line 319 of StationaryButtonDecorator.java
which adds <div style='overflow: auto;'>
firefox_1R1sIn9v3X.pngfirefox_BHZ0ARau9r.png

Looking at possible other values for that line:


'' (<nothing>): expands the previous scrollbar.
---auto: gives an extra scroll-bar
---hidden: doesn't let you scroll to the overflow content
---inherit: parent already seems to be auto, so also gives the extra scroll-bar
initial: expands the previous scrollbar.
revert: expands the previous scrollbar.
---scroll: the scroll-bars are always there, even when there's nothing to scroll to (at least wouldn't cause the "glitching" mentioned earlier?)
unset: expands the previous scrollbar.
visible: (the default) expands the previous scrollbar.
----moz-hidden-unscrollable: like hidden (also unique to mozilla, I think?)

So, (<nothing>), initial, revert, unset and visible are all possible solutions.
I'm not very good at CSS, so I'd let someone more qualified to tell which is right, but I think that:
unset, initial and visible are all the same, since it must just leave it to the default, which is visible,
(<nothing>) and revert are also the same, since it just takes the previous value. Also, since there does not seem to be any "previous" value, these two just point back to the default, which, again, is visible.
So they are all good, because they simply all point to the same one, which is visible?

So the answer would either be <div style='overflow: visible;'>, or just... <div>


(disclaimer: I have NOT tested these things by modifying mafia itself; I only played with the element inspector. Same goes for the post above this one.)
 

fredg1

Member
Update: inherit seems to also be what causes the cropping in such situations:firefox_rhyg296Gd4.png

This is the only one that does, so this leads me to think that "auto" is defaulting to "inherit" in most situations, here.
 

fredg1

Member
Ok, I've been trying to help by testing overflow: visible, and I've... had some problems..?
Code_lO8zsD6d1Z.png

I compiled a new build in which I made the change I showed, but for some reason it's just... not showing in the actual page?
Is this the wrong place? This seems like it, though, as it's the only place that succeeds to a div with ID: "content_", and what's before that also seems to match..?

Is that saved in the Build folder or something? What am I getting wrong here?
 

xKiv

Active member
but for some reason it's just... not showing in the actual page?

If you are changing a .js file, you need to take into account browser caching. And, iirc, javascript files are cached way more aggressively than everything else, to the point that even clearing cache (often?) doesn't remove them from the cache. This is why those files are versioned by changing their name.
I *think* you can force loading current version of a .js file by accessing it through a direct url in a new tab and ^R repeatedly.
 

matt.chugg

Moderator
If you are changing a .js file, you need to take into account browser caching. And, iirc, javascript files are cached way more aggressively than everything else, to the point that even clearing cache (often?) doesn't remove them from the cache. This is why those files are versioned by changing their name.
I *think* you can force loading current version of a .js file by accessing it through a direct url in a new tab and ^R repeatedly.

Can the script tag be adjusted, if you set the src attribute of the script tag to include a querystring variable and dynamic value it will prevent the browser caching it.

I don't know the syntax in Java but in PHP I use microtime()

Code:
<script src="some_js_file.js?version=<?php echo(microtime()); ?>"></script>

microtime is in miliseconds so will change everytime the page is loaded and never cache, useful for development environments, it would probably be sufficient to simply use the build number so the JS is refreshed once for every new build


As a secondary useful bit of information, Chrome has a tickbox to disble the cache in the network tab of the dev tools window. I'm certain other browser will have similar
 
Last edited:

fredg1

Member
Ok, found the issue: turns out I was simply mistaken; the part I was editing wasn't the fight.php CAB at all! Rather, it was the place where the "fake" CAB was built, for choice.php and adventure.php
firefox_NWe1uYDU6x.png


The part that matters is, instead, in THIS section: Code_y5GTRMz94S.png

The change I made is still beneficial, though, for this reason: xgtQi8RAW8.png vs nUMB4pTUMj.png, so I'll keep it, but now I know that to fix the glitch I was targeting, I need to actually modify the current page.
 

fredg1

Member
Did you, by any chance, test this with the CAB on and CAB off?

That's... unnecessary?

The edits I made (...and the issue itself) are limited to the part of the code that's reached if serverAddsCustomCombat is true (which is set based on the "Enable Combat Action Bar" KoL setting)
jH7RzGKitH.png


The part accessed if it's false simply adds an attribute-less div to the page (which, I'm pretty sure, defaults to overflow: visible, which is just what we want):
YCQqNr3ePd.png

This means that the problem was never present to begin with if you had serverAddsCustomCombat = false (which is in fact confirmed by Veracity saying she never encountered that issue, while never using the CAB).


TL;DR, "testing" the CAB off gives nothing: the edits don't affect that state, and the problem isn't/wasn't there either.
 

fronobulax

Developer
Staff member
I asked because if this does break anything it is my name on the commit and it is easier to ask :)

r20383 Sorry I hit return instead of a commit message crediting you.
 

MCroft

Developer
Staff member
So I tried it (from the patch file, so if it's materially different in the checkin, I'll need to retest).

First image is with the combat bar (my normal operating mode). It seems fine.
Second is with the combat bar off (I've never used this before). It seems ... mostly fine. There's text in one of the macro dialogs that I'm not used to. I don't know if that is normal.
Third image is with the combat bar off. It seems mostly fine. I have a lot of stats that will change, so it's pretty good for checking. It still cuts off the last bit...

Yes Combat Bar (still cut off?).png
No Combat Bar 1.png
No Combat Bar 2.png
 

MCroft

Developer
Staff member
I don't know if this is new, but now that it's overflowing, maybe give it a CSS property to put it in front of the Palindome?

Screen Shot 2020-09-22 at 11.44.15 PM.png
 

fredg1

Member
So I tried it (from the patch file, so if it's materially different in the checkin, I'll need to retest).

First image is with the combat bar (my normal operating mode). It seems fine.
Second is with the combat bar off (I've never used this before). It seems ... mostly fine. There's text in one of the macro dialogs that I'm not used to. I don't know if that is normal.
Third image is with the combat bar off. It seems mostly fine. I have a lot of stats that will change, so it's pretty good for checking. It still cuts off the last bit...

The image that seems to have something cut off is the first, though? You say it's the third...
Also, what do you mean by "still" cutting off? All I saw before was an extra scroll-bar being added to reach the added portion; it was always "somehow reachable", never "cut"..?

How is it cutting? Is it abruptly?
Can you try inspecting the element to find out more about this?
 
Top