User tests: Successful: Unsuccessful:
Pull Request for Issue #16968.
Change params variable to reflect merged state (in address sub-layout).
See #16968 (comment)
The mentioned field values and icons are shown to the user.
They are not.
Status | New | ⇒ | Pending |
Category | ⇒ | Front End com_contact |
I have tested this item
It seems drone failed for reasons not related to this PR (some phpcs script missing).
I saw the bug after updating. Gave it a try by exchanging my override with this new version in three different websites. Works fine now :-)
Status | Pending | ⇒ | Ready to Commit |
rtc
I think we need same edit also in the template beez override
templates/beez3/html/com_contact/contact/default_address.php
hm, if all template overrides have to be updated, isn't it a b/c break then? in other words: something was changed and now breaks template overrides.
if all template overrides have to be updated, isn't it a b/c break then
Not according to the B/C promise:
6.1.7 Rendered markup
For the time being, rendered markup is not subject to our backwards compatibility promise. We will try not to change markup in such a way that a site might render differently, but we can't promise not to break anything at the present time. We will work on defining ways in which we might make a backwards compatibility promise for markup in the future, but we do not currently have a satisfactory consensus on a workable standard.
ah, ok. thanks for the info!
@dgt41 I am affected, but I'm not sure why. Maybe I'm doing somthing wrong in a plugin I created, which adds some fields to the contact edit form in backend. These fields are saved with the params of the contact.
In my template overrides I was able to get these params with
$this->params->get('paramname'),
but now I have to use
$this->item->params->get('paramname') or $this->contact->params->get('paramname').
This only applies to the "custom" params, the "core" params work as before.
Now I'm confused :-) But maybe I should just add my fields in a different way.
Edit: And yes, my problem is not related to this PR
Status | Ready to Commit | ⇒ | Pending |
I'm switching back to pending based on a special request by @matrikular
First of all, I feel responsible for this and do want to make it right. You can be assured, that it is not my intention to hide behind some "yet unwritten", "debatable", "between the lines" interpretation of the stated b/c promise regarding rendered markup.
That said, personally - I would expect every site owner / admin / webmaster to thoroughly check their overrides after each update and that a layout is hardly to keep free from "breaking" changes. A simple change in the data layer or parameter handling e.g. can very well affect the output.
So much for cheap excuses,...
I also think, in case of the address layout, it would be (already is) a b/c break in a way that, everyone that already has got an override, would need to update it because of the "new" variable being used. This is because in different from the default and custom fields layout, it uses $this->params variable directly, where the other two accessed the parameters via the variable $tparams or $params, therefore no layout wide renaming was needed.
I've reverted the changes back to the original state for the address layout and assigned the merged parameters to $this->params in the view. That way, there should be no need to change an already existing override. Well, at least not in this context (hopefully).
Some more tests are highly appreciated, thanks.
I have tested this item
@matrikular Just when you had posted your previous comment I have finishing my test ;-)
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC after two successful tests.
I have tested this item
Fixes problem mentioned in #16968 and custom overrides.
I have tested this item
I tested this on four sites that had this issue and all tested great with the fix provided. Thank you!
Jenn
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-07-06 07:33:58 |
Closed_By | ⇒ | rdeutz | |
Labels |
Added:
?
|
I have tested this item✅ successfully on 8d0e5f5
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16971.