As described on the task, floor() returns the closest integer to the
left, and so it's not the right choice in this case for negative
offsets.
Put the logic in a static method of the UserTimeCorrection class so that
it can be reused elsewhere without making the previous mistake, and add
tests for it.
Also update a comment in UserTimeCorrection, as a follow-up to
I99a00dff7e3319ce45883191daee16bec1ed68ba.
Bug: T318455
Change-Id: I9acc8fa278d5a58a1d56c28c9e8b3f9093f8add9
This patch simplifies and fixes a few issues in code related to the
'timecorrection' user setting:
- Always re-apply UserTimeCorrection normalization to the preference
value, although it should be already normalized
- Avoid duplicating code from UserTimeCorrection, both for the
pipe-splitting and the offset computation/fallback
- Use better variable names
- Inject an ITextFormatter for generating the dropdown options, instead
of a ContextSource (ew) or a Language, removing calls to wfMessage as
well. Note that the ITextFormatter is not injected into the
preferences factory because the eventual goal is to move the code to a
new HTMLFormField class.
- In TimezoneFilter, remove a redundant check: the value comes from the
form, and the option for using the system time is always "System|XXX",
never just "System". This seems to have been introduced in
I2cadac00e46dff2bc7d81ac2f294ea2ae4e72f47; the previous code was only
comparing $data[0], and not $tz. Change the test accordingly and add a
test case.
- Add missing star to docblocks in UserTimeCorrection, as well as a
missing int cast.
- Fix typo and other style issues in UserTimeCorrectionTest
- Bonus: add missing docblock star in HTMLApiField
Bug: T309629
Change-Id: Iab35eb17259826429e4b6bc1ba7385ab57884e98
The constructor param $offset was assigned to $this->offset, but this is
useless and confusing. parse() is called immediately, and it always
overrides $this->offset with a value derived from the $timeCorrection
string, *unless* the correction type is SYSTEM. So rename the parameter
and make it clear that it's only used for SYSTEM.
Also add a FIXME about offsets after "System|" are ignored. This looks
like a leftover from older MW versions, and it's already noted in
SetupDynamicConfig.php, too. However, I'm too scared to remove it,
because I fear it could break something (especially because I've no idea
what the values in the DB could be, considering the other FIXME in the
parse() docblock).
Change-Id: I0156d5f7824f87ba6bc7f395135a4e1c80c517f4
Don't catch and discard exceptions from the RequestTimeout library,
except when the exception is properly handled and the code seems to be
trying to wrap things up.
In most cases the exception is rethrown. Ideally it should instead be
done by narrowing the catch, and this was feasible in a few cases. But
sometimes the exception being caught is an instance of the base class
(notably DateTime::__construct()). Often Exception is the root of the
hierarchy of exceptions being thrown and so is the obvious catch-all.
Notes on specific callers:
* In the case of ResourceLoader::respond(), exceptions were caught for API
correctness, but processing continued. I added an outer try block for
timeout handling so that termination would be more prompt.
* In LCStoreCDB the Exception being caught was Cdb\Exception not
\Exception. I added an alias to avoid confusion.
* In ImageGallery I added a special exception class.
* In Message::__toString() the rationale for catching disappears
in PHP 7.4.0+, so I added a PHP version check.
* In PoolCounterRedis, let the shutdown function do its thing, but
rethrow the exception for logging.
Change-Id: I4c3770b9efc76a1ce42ed9f59329c36de04d657c
The parsing of the timecorrection useroption was split over multiple
classes. Combine into a single class and add some testcases.
Change-Id: I2cadac00e46dff2bc7d81ac2f294ea2ae4e72f47
