Set the device-phone CSS class also for Firefox on Android to (hopefully) avoid or decrease large layout shifts.
See https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/User-Agent/Firefox
Closes T16071
Details
Details
- Reviewers
- None
- Group Reviewers
O1: Blessed Committers - Maniphest Tasks
- T16071: Avoid large layout shifts while page loading on mobile
Check if the given regex matches User-Agent strings.
Diff Detail
Diff Detail
- Repository
- rP Phorge
- Branch
- T16071FFmobileUA (branched from master)
- Lint
Lint Warnings Severity Location Code Message Warning src/view/page/PhabricatorStandardPageView.php:630 TXT3 Line Too Long - Unit
Tests Passed - Build Status
Buildable 1980 Build 1980: arc lint + arc unit
Event Timeline
| src/view/page/PhabricatorStandardPageView.php | ||
|---|---|---|
| 630 | In Chrome the "Mobile" is after the version so it's more complicated, but in Firefox seems easier, we can just match "Mobile.*Firefox" probably P.S. feel free to implode by | and have an array if it helps in getting rid of the linter | |
Comment Actions
but in Firefox seems easier, we can just match "Mobile.*Firefox" probably
I prefer that more specific regex as you never know which completely random user-agent strings are out there. It's consistent with the Chrome pattern.
| src/view/page/PhabricatorStandardPageView.php | ||
|---|---|---|
| 630 | Thanks. In case you don't like Mobile.*Firefox (that was proposed just because it's simpler and shorter and faster) at least evaluate Mobile.*Firefox/ (that is still simpler and shorter and faster but more strict with that added slash, since the specification says that there is that slash). (This is just a non blocking tip - the block is because people have to test lol) | |
Comment Actions
There is something wrong now nearby the |((Chrome, that should be enclosed by pipes and just one bracket, like an hamburger of pipes, like @stuff|(Chromestuff)|(Firefox stuff)@
Premising that I do not understand the old legacy code, that could be this, without brackets:
@iPhone|iPod|Android.*Chrome/[.0-9]* Mobile@
So it should really work with just this without the slash:
@iPhone|iPod|Android.*Chrome/[.0-9]* Mobile|Mobile.*Firefox@
or with the slash:
@iPhone|iPod|Android.*Chrome/[.0-9]* Mobile|Mobile.*Firefox/@
or with slash and version:
@iPhone|iPod|Android.*Chrome/[.0-9]* Mobile|Mobile.*Firefox/[.0-9]*@
Comment Actions
e.g. https://regex101.com/r/PDyAGh/1 (then the slash, or the slash and the version - but that is)
Comment Actions
Right...hmm, now I also wonder. :)
Should probably be iPhone|iPod|Android.*(Chrome/[.0-9]* Mobile|Mobile.*Firefox/[.0-9]*) (it's still that Firefox on Android has that Android string first, thus the brackets).
or with slash and version:
We shall keep version. There"s a good number of user-agent strings out there concatenating random browser names while not being these browsers.