This click handler is assigned to many different elements. Not all
of them can be used as a source for a data-pos="…" attribute. But a
lot can.
Again, this patch alone will not actually fix T352169. But it will
improve the situation a lot, according to my local tests. Many of
the clicks will start to work fine on RTL wikis because the
problematic pixel-based getRevisionPositionFromLeftOffset
calculations are not used any more.
Steps to reproduce: Go to a RTL page with a very long history, e.g.
https://he.wikipedia.org/wiki/Special:Diff/38031767?uselang=he
Click on one of the gray bars in the right half of the slider. This
will not work, i.e. the slider will not move to this position but
to a totally different one. This is the bug described in T352169.
I noticed that a browser zoom other than 100% can cause many more
problems that are unrelated to this patch. Please test with 100%.
Bug: T352169
Change-Id: Ife49557c891736bc94df6087658f76326791f61b
This updates the code a little bit to be closer to the current
version of https://github.com/othree/jquery.rtl-scroll-type
without actually changing anything (for now).
I tried copy-pasting the current version to see if that helps with
T352169, but it doesn't make a difference.
The only actual change is that the dummy element is removed when
it's not needed any more.
Change-Id: Ibdd064e228fa9464a8652ce9a8a9ac388662f29d
I have seen this getting fed with fractional numbers like
38.9994375. This apparently happens when specific browser zooms
are used, e.g. 110% in Google Chrome. There is a lot of code that
expects this to be an integer number and stops working entirely
when it isn't.
I'm also updating two related comparisons to not be so extremely
specific any more. This probably doesn't make a difference any more
with the fix above, but can't hurt.
This patch doesn't solve T352169, but is one more puzzle piece on
the way to solve it.
Bug: T352169
Change-Id: Ied0b9748beec941e901ca4ecba428c16967ca510
The version numbers mentioned in the comment are from the V8 engine.
The last Google Chrome version to use a 5.x version of the V8 engine
was Google Chrome version 59. That was 8 years ago. See T168299.
Our official support matrix asks for 3 years.
https://www.mediawiki.org/wiki/Compatibility#Browser_support_matrix
Notice how there is no version number in the code despite the comment
explaining that it shouldn't be used in later versions.
According to my local tests this is not a full fix for T352169, but
notably improves the situation. I can still see the bad behavior
described in T352169, but only in a narrow region on the right side
of the slider. Removing the obsolete browser detection is necessary
to unblock further investigation.
Making the dummy text a bit longer also makes an actual difference.
To keep this patch as minimal as possible all I do is to add a single
character.
Bug: T352169
Change-Id: I56f3c1969ce4f164f4319e5038d0f97527e6b1c0
The method is only called from two places, and both do very similar
things before the call. This duplication can be merged.
One notable difference is that .scrollLeft() was possibly called
twice before. The first call was pointless anyway. The argument for
.scrollLeft() is absolute, not relative.
This code cleanup is motivated by T352169, but doesn't solve it.
Bug: T352169
Change-Id: I75e9ffc77ef6331f14e074921c78c28233e60840
This was almost the exact same code, with very few differences (e.g.
different CSS classes, and different icons).
Another change in the same file is the removal of an extra
`!== undefined` check. This is impossible. I can't tell why this was
there. Maybe an artifact from when this was developed?
This code cleanup is motivated by T352169, but doesn't solve it.
Bug: T352169
Change-Id: I3bb7ce00f9b754f9ba58310100b855c8ee3fca4a
The latest update of 'svgo' dependency includes three optimizations on
converting path commands, which
- improves closing paths and how we determine if to use absolute or
relative commands.
- round arc or convert to lines based on the geometric sagitta
- convert cubic Bézier curves to quadratic Bézier curves where possible
Also unifiying npm command to qua standard notation `minify:svg`.
Bug: T354875
Change-Id: I38ccbfa62ee7afcfb10eee7853b33648863f54ad
User-options related classes are being moved to
the MediaWiki\User\Options namespace in MediaWiki Core;
reflect that change here.
Bug: T352284
Depends-On: I42653491c19dde5de99e0661770e2c81df5d7e84
Change-Id: I5b9f4c7ea90f492f75b9055b801ec0853da22687
User-options related classes are being moved to the MediaWiki\User\Options namespace in MediaWiki Core; reflect that change here.
Bug: T352284
Depends-On: I9822eb1553870b876d0b8a927e4e86c27d83bd52
Change-Id: I4daca7542a428455aa72cc372521e894716e3c40
Notable:
* Arrays shouldn't be initialized like this. Instances will actually
share the same array object. Luckily this was dead code anyway
because it's re-done in the constructor.
* $timeOffset is already guaranteed to be an int.
Change-Id: Ib0a2b0f39ee368fcef4756281099d519d470eb44
This utility method doesn't really belong here. This does at least
reduce the surface area. Now it's only a single method instead of
two.
Change-Id: I21fbb4e4922f2cc3bc3e23c457b5ceeb216f3ab4
Turns out we can run into this failure situation even if everything
goes according to plan. It is possible to click just one pixel outside
of the valid range, e.g. on the very right corner right next to the
last bar. This is supposed to not do anything anyway, and correctly
doesn't do anything from the user's perspective. But it shows up as a
failure in the JavaScript console.
Bug: T352169
Change-Id: I12c9cce90970be36667ba1b721afd38a13a063c9
This is a manual revert of I06b8af8. Turns out the list of revisions
(and with that the maximum size calculated from the list of revisions)
changes during the lifetime of the view. It must be recalculated.
Maybe not as often. But this is an optimization for a later patch.
Change-Id: Id1dce5566a9041396bcf72df7e4936be0e53188a
When expanding the slider to newer or older revisions the handler
for revision clicks should only be added to the new revisions.
Change-Id: If590996d27dc75cbdfc931e9649418f875c3869d
We added this more as an experiment because we couldn't reliably
figure out what makes later .offset() calls fail. After looking into
the actual jQuery source code I realized the only way .offset() can
return undefined is when .length is zero. Which means the two checks
are redundant and one can be removed.
In case an element is not attached to the document an object with
{ top: 0, left: 0 } is returned.
Bug: T342556
Change-Id: I6265fd27b3102a9cfe853a9c0e11063b76cf0b7b
Otherwise the `this` to call addColoredColumnBorders() inside of
that function is unkown when triggering the callback.
Can be verified working when collapsing the slider on master vs
this patch.
Regression introduced in Icffe9551d633470ccec1b63ea570e138db48dee8
Change-Id: I653db133688d7678d0cb3b80936c7e9a7ebebd1f
This is a first step in restructuring and consolidating the
highlight and tooltip creation. I use revision focus to communicate
the state when a revision is highlighted and shows a tooltip.
There's still stuff to untangle but I want to keep the diffs small.
Change-Id: I0b169042837a2c3bb825c23368e7e8a485694eb5
