User tests: Successful: Unsuccessful:
Labels |
Added:
?
|
That is off topic for this issue
On 10 February 2015 at 11:38, waader notifications@github.com wrote:
@test https://github.com/test works for me!
@brianteeman https://github.com/brianteeman When testing this patch at
first I could not find the respective configuration setting. Then I looked
in the code and found it behind the term "Mouse-over Edit Icons for". I
always watched out for "Frontend"-something.Also the option terms were not clear to me. It states "Module" when I
could also edit mod_menu and "Modules & Menus" when mod_x and Menu Items
are meant. So "Modules & Menu Items" would be more precise.—
Reply to this email directly or view it on GitHub
#6041 (comment).
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
but it might be good to fix the wording prior to 3.4.0 and add a post install message that gives notice regarding the performance hit when this is enabled.
@brianteeman I don´t think so because this feature is already advertised and some people will want to try it out. When it´s activated by default, you will not have to find a configuration setting. Otherwise you have to. Anyway.
I guess you didnt realise then that this text has been present since 3.3 at
least. The change is that the behaviour is now to open the module in the
frontend not the backend.
On 10 February 2015 at 11:57, waader notifications@github.com wrote:
@brianteeman https://github.com/brianteeman I don´t think so because
this feature is already advertised and some people will want to try it out.
When it´s activated by default, you will not have to find a configuration
setting. Otherwise you have to. Anyway.—
Reply to this email directly or view it on GitHub
#6041 (comment).
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
Be aware that this paremeter isn't a new feature and it already exists already in 3.2.
Changing the default value thus will have an impact on existing sites and remove the frontend editing for sites where it was enabled before.
That is in the case where the global configuration wasn't saved since 3.2.0.
Thus it isn't a backward compatible change. Although minor.
It is not something that breaks a site. It also isn't something that changes anything for Google or any normal user. You have to have quite some permissions to use this feature. Regardless of what we do with this PR, we should add a post install message telling people of the very negative effect this feature has on their performance.
I thought With your latest pr there is no negative effect
On 10 Feb 2015 13:50, "Hannes Papenberg" notifications@github.com wrote:
It is not something that breaks a site. It also isn't something that
changes anything for Google or any normal user. You have to have quite some
permissions to use this feature. Regardless of what we do with this PR, we
should add a post install message telling people of the very negative
effect this feature has on their performance.—
Reply to this email directly or view it on GitHub
#6041 (comment).
Please not yet another postinstall message.
And we indeed should fix the negative effect (which the other PR should do).
As I wrote, that PR only solves the issue for unauthenticated users, users that are not logged in. All users that ARE logged in, will still have all these checks and there is nothing that we can do against that, except switching off the feature. Which is why I'm proposing to setting this to not enabled by default. For all community websites, this will still mean quite a performance hit. For all webshops where you login at some point, this still means a performance hit. So, no, #6032 does NOT fix this issue. It fixes this for all static websites out there, but the issue remains. Which is why I proposed this PR. And which is why I proposed to not allow to select this for single modules, but all or nothing.
P.S.: Did anybody notice how I always used "not enabled" instead of "disabled"? I fear that this gets shot down so much just for the language that I use, that I desperately try to use positive words, because otherwise the B/C-police will call to the rescue again to help all those 5 poor souls that need this enabled, but can't find the option in the global configuration. I mean, of course it is better to reduce the speed of 99% of all websites made with Joomla out there instead of making that 1% that might be using it search for that option.
And of course we should tell people that we pushed a feature into Joomla that is enabled by default, that slows down their site, that they are most likely not using and that can be switched off by setting it like that in the global configuration.
Watching your discussions for a while - and highly interested in usability and performance and clean code I want to give my2cent. Wouldn't it be better to say frankly "For a better performance we have decided to disable this feature - if you really need it - be aware that it affects the performance". Users are notas supid - they know that bad performance is bad for their sites.
Backward compatibility is important, indeed. This makes it difficult. But I'm sure that you can communicate this.
Watching your discussions for a while - and highly interested in usability and performance and clean code I want to give my2cent. Wouldn't it be better to say frankly "For a better performance we have decided to disable this feature - if you really need it - be aware that it affects the performance". Users are notas supid - they know that bad performance is bad for their sites.
Improved performance is also an important feature, one that many would rate higher than front-end editing.
Easy | No | ⇒ | Yes |
Category | ⇒ | Front End Modules |
@Hackwar since this is RTC: #6032 do we still need this change here or can we close this PR?
Please see my comment above. This PR is still definitely needed.
This PR still isn't B/C.
I strongly disagree. By your logic, the recaptcha plugin would not be B/C either. We are disabling a feature on websites that did NOT enable it in the first place. So in fact, you forced this on people somewhere in 3.2 and now I'm proposing to let people choose instead. It does not break anything and it does not take a feature away. It only makes 95% of all current Joomla websites 20% faster.
Why would the recaptcha plugin not be BC? It defaults to the old API afaik.
Yes, we had that feature enabled by default, which was probably wrong back then. But you propose to disable it now while it worked for all sites during the last two minor releases.
It only makes 95% of all current Joomla websites 20% faster.
That's absolutely not true. With your other PR it only affects sites where a user is logged in. Which isn't 95% of the Joomla installations for sure and by far.
The performance increase is also not 20%. The reported increase is 20% for the database queries, which is not the full website. On my crappy testing server at home, it accounts for about 2% performance gain. It obviously will have different gains depending on server, caching and website.
Together with the other improvements, it will be an even smaller gain.
So please don't try to get this PR merged by using false facts. That will not help your case.
Let me give you another example: We changed the password hashing method in between minor releases, even worse, we changed them again in a patch release. That actually broke extensions and several websites and it broke them in the worst possible way. That apparently was okay. Fixing this for some reason is not okay, even though it does not break any extension, any website or any feature.
The performance improvement when this is disabled on my system is a lot bigger than 2%. I didn't calculate it exactly, but remembering the numbers it should be somewhere around 10%. However, the exact amount doesn't matter one bit.
What matters is, that this feature, the way it is, adds noticeable load on websites and even with my fix in #6032, this additional load is still present for all logged-in users. Yes, you can switch this feature off and everything would be fine, but you would have to first of all know about this feature and that it has a negative impact on your performance and then know how to switch it off.
Now, I claim for myself that I know Joomla pretty well. I used it here or there a tiny, teensie bit and might have read a few anouncenments and a bit of source code [sarcasm off] and I had NO idea that this feature had negative performance effects for everybody and I didn't know that you could switch it off. And even after I investigated the source of the performance problem described in #5996 and provided a preliminary fix for it in #6032, I still didn't know where to switch this off. I had to look into the code of com_config to learn that "Mouse-over Edit Icons for" means that I can edit modules in the frontend. The description "Select if you want mouse-over edit icons for modules and menu items (support may depend on your template)." is equally useless. The german label is equally confusing, while the description is at least a tiny bit better, but you would still have to know quite a bit about the system to make anything good out of that. The french translation is mostly identical to the english text.
So, again, there is a feature that people know very little about, that is currently broken in 3.3 in terms of performance, where we only have a bandaid solution for 3.4 and whose off-switch is hidden so good that we might as well remove it altogether and nobody would notice.
This is not a break in backwards compatibility. That would assume that something is broken after this change. That is the definition of backwards compatibility and breaking it. Here we are talking about a change in functionality. These happen all the time and are expected. So please don't claim that this is a break in compatibility.
That apparently was okay.
Not it wasn't. And it was one of the reason we changed our whole strategy.
The performance improvement when this is disabled on my system is a lot bigger than 2%. I didn't calculate it exactly, but remembering the numbers it should be somewhere around 10%.
As said, it may differ a lot between enviroments. Depending on how fast the database is, the harddisc, the PHP stack, how many modules there are, how many other database queries from extensions, stuff like that. But it's certainly not 20%. On the other hand I agree it's noticable on most websites where users are logged in.
That is the definition of backwards compatibility and breaking it.
That may be your definition. It's certainly not mine.
Taking your statement that this switch is so well hidden, how do you expect the users who used this feature now for two minor release to find the switch to enable it again?
In my opinion it's a backward compatibility break and if we are going to make an exception and merge this, I want people to be aware that this will cause support requests for sure.
I would like to have the other possibilities (like your other PRs) tested and merged first.
If the performance issue is still big enough after all the other improvements, then I'll leave it up to @wilsonge to make that decision to create a minor B/C here.
So because the language string is total bullshit, we can't change this? I'd rather create yet another PR to change that language string to something sensible, than to drop this PR. I also already proposed a post-install message that makes people aware of this.
The argument "Our interface does not label that switch correctly, so we can't set it to the value that it should be" is no argument for me.
The argument "Our interface does not label that switch correctly, so we can't set it to the value that it should be" is no argument for me.
It was your argument that the switch is hard to find. Not mine.
No, my argument is, that the switch is so hidden and impossible to find, that even if people knew about this feature and the consequences of this feature, that they were unable to disable it. You drew the conclusion that we could not disable it then, because people would be unable to enable it again. The same reason that I would buy a new car every 2 months because the ashtray is full if I were smoking. The correct conclusion would be to make it simple for people to find the switch by improving the label and adding a post-install message.
I have tested this item successfully on f0267e3
Very difficult to find the functionallity, but finally it worked
I have tested this item successfully on f0267e3
Tested successfully
Joomla 3.4.5
Chrome
how can I get access again . under which menu item . so not ok may be given
It's an option in global configuration
we still have the icon at the postion " Recent Posts " Archive Articles etc. The patch works only with items.
The menu item for the A and Blend is very confusing deposited . Does not quite fit . I would put it under Article Options purely
Milestone |
Added: |
||
Status | Pending | ⇒ | Ready to Commit |
Milestone |
Added: |
Labels |
Added:
?
|
Milestone |
Added: |
Milestone |
Removed: |
Let's make a decision on this, I am going to ask the PLT to vote on whether or not to accept this PR. If it is accepted I will merge it for 3.5 otherwise I will close it.
Milestone |
Removed: |
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-11-14 20:44:10 |
Closed_By | ⇒ | roland-d | |
Labels |
Labels |
The PLT has decided not to change this behavior but that the language string needs updating. I have made a suggestion for that in the PR #6105
Thanks everybody for your contribution.
Labels |
Removed:
?
|
@test works for me!
@brianteeman When testing this patch at first I could not find the respective configuration setting. Then I looked in the code and found it behind the term "Mouse-over Edit Icons for". I always watched out for "Frontend"-something.
Also the option terms were not clear to me. It states "Module" when I could also edit mod_menu and "Modules & Menus" when mod_x and Menu Items are meant. So "Modules & Menu Items" would be more precise.