Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
This one occurs only on Firefox on soft reload (ctr+r / cmd+r) and is it a regression issue as the problem does not occur in D7!
Steps to reproduce:
- Add an new field to a node entity e.g. "Text (formated, long) with cardinality=unlimited."
- Navigate to node-edit of a node entity with the new field.
- Click "Add another item".
- Execute a soft reload (ctr+r / cmd+r).
- Click "Add another item" after the page has been sotfly reloaded.
- Now watch how instead of one new item there will be two new items generated.
Similiar problem is also that on soft reload the user input remains on the reload page like filled text fields and checked checkboxes. This does not occur in D7.
Proposed resolution
None at the moment.
Remaining tasks
- Find out why there is this difference between D7 and D8.
- Find a way to bring back the previous behaviour.
API changes
None at the moment.
Comment | File | Size | Author |
---|---|---|---|
#19 | 2596597-8.patch | 1.26 KB | claudiu.cristea |
#16 | 2596597-16-turn-off-form-autocomplete.patch | 1.24 KB | hchonov |
#8 | 2596597-8.patch | 1.26 KB | sardara |
Comments
Comment #2
hchonovComment #3
hchonovComment #4
cilefen CreditAttribution: cilefen commentedCould you check the priority level on this issue? I am not sure it is major priority. If this applies:
.. then it is major.
Comment #8
sardara CreditAttribution: sardara at Randstad Digital for European Commission and European Union Institutions, Agencies and Bodies commentedWe faced this bug in our project too.
The issue is caused by the Firefox functionality that restores form data on a soft reload (see a Stackoverflow issue reporting the same).
This unfortunately happens also for the
form_build_id
hidden input, which is used to point to a specific cached version of the form build.So, from a fresh load of a form page (where a field with cardinality N is present):
form_build_id
is populated with a value, let's sayform-1st-build-id
;update_build_id
command, which is going to replace the old build id with the new one, let's sayform-2nd-build-id
;form-totally-new-build-id
;
form-2nd-build-id
value;In our project we have a more complex form, with inline_entity_form fields and field groups, and the results were even more severe, having the wrong widgets rendered in place of the inline entity forms.
As explained in the Stackoverflow issue, this can be solved by setting the attribute
autocomplete="off"
to a field.So my suggestion is to add this attribute to the
form_build_id
input.This is very hard to be tested, as it doesn't happen on Chrome and Opera, so it needs manual testing.
Comment #9
pfrenssenSome good sleuthing here and it seems to resolve the issue. It would still be a good idea to figure out why Drupal 7 is not affected. Does it keep the same form build ID when AJAX requests occur?
Comment #10
hchonovThis is a pretty nice find and solution, @sardara!
I guess this is just a typo and the form build id will be restored back to the second one, right?
Comment #11
hchonovI've just done a manual testing and the the patch fixes the problem. But as we are able to write Javascript tests now, maybe it will be possible to reproduce the issue in a test?
Comment #12
pfrenssenThat's quite unlikely since we are using PhantomJS which is based on Webkit to run the JS tests. The issue only occurs on Firefox.
Comment #13
sardara CreditAttribution: sardara at Randstad Digital for European Commission and European Union Institutions, Agencies and Bodies commentedThanks @hchonov, you are right. I fixed the comment.
Comment #14
sardara CreditAttribution: sardara at Randstad Digital for European Commission and European Union Institutions, Agencies and Bodies commentedIn Drupal 7 the form build id gets replaced only when "the cached form was rendered on a cacheable page" (includes/form.inc:540 and includes/ajax.inc:344). Drupal pages are uncached for logged in users or when a session is present (includes/session.inc:252).
Comment #15
hchonovWell with this patch we fix the problem for multi step forms. The fix and this change have to go in for sure.
But still we have a difference between Firefox and Chrome e.g. go to entity/edit, alter the title field, soft reload the page - in Chrome the input value is the old one and in Firefox the changed one before the page reload.
If you have big and complex forms and if a user does soft reload thinking all the changes will be lost and then makes further changes and submits it will be a mess. Should we disable autocomplete at the form level to cover this? Probably we don't want to do this, but the problem with having mixed input after soft reload sounds worse than not having autocomplete at all.
Comment #16
hchonovWe could turn off the autocomplete for all input elements at the form level. Still not sure if this is the right thing to do..
Comment #18
sardara CreditAttribution: sardara at Randstad Digital for European Commission and European Union Institutions, Agencies and Bodies commented@hchonov, the autocomplete functionality is provided by the browser and it's something that Firefox users expect from it. I think that disabling this completely in Drupal by default should be avoided, and it should be something project-specific maybe.
For example, in Firefox you can reopen a tab closed by mistake and get the simple fields filled back. If we turn this off on form level, the user will loose an expected functionality. It's the responsibility of the user to check the input before clicking a submit button.
Comment #19
claudiu.cristeaRemoving all autocomplete functionality is a UX regression but disabling it only for
form_build_id
sounds OK for me because it doesn't bring any regression to user interaction. It's just something hidden. This looks like a nice workaround.I'm reposting the patch from #8 just for RTBC. This is good to go, IMO.
EDIT: It was manually tested by @hchonov and by me.
Comment #20
pfrenssenI also tested manually with Firefox 53.0. I can also confirm that the patch solves the issue. I am very impressed with the detective work that resulted in this simple solution, huge thanks @sardara!
Comment #21
hchonov@sardara, @claudiu.cristea I agree with both of you, but we still have to decide what we are going to do about soft reloads in Firefox. I think we have to somehow achieve it that on soft reload the form does not get refilled with the previously changed values.
Probably disable autocomplete initially on the GET-Request and then in an attach behavior enable it - in this case Firefox will not refill the changed values back again and this way we'll not introduce any UX regressions, right?
Comment #22
pfrenssenWhy do you think it would benefit users to disable this functionality? I think disabling this would be a bad usability regression. I personally rely heavily on this feature with some software that is very fragile from a usability standpoint (e.g. Jira with all its UI trickery). It's great to be able to make a mistake and returning back to the previous state without losing all my unsaved data.
BTW this is a feature that most modern browsers have: Chromium also restores form input when a form is reopened (e.g. by navigating to another page and then pressing the back button), but Chromium only replaces the visual form input elements and leaves the hidden
form_build_id
input alone, so it is not affected by this. On Chromium a soft reload clears the form state but that's browser specific and within the expectations of the browser's user base. It works fine when navigating away and back, or closing and reopening a tab.If you _really_ want to clear your form state you can always do a hard reload or revisit the URL. This is consistent with normal browser behaviour.
Comment #23
hchonov@pfrenssen, I am only talking about soft reloads, not going back - both are different things. And in Chrome the user input is not being refilled on soft reload.
Think about a one really big form structure .. you make a lot of changes but then decide you want to start from scratch .. you perform a soft reload and with Firefox you might not notice that actually you are going to save the changes you wanted to throw away. And honestly how many users know what a hard reload is? The reload button on each browser performs only a soft reload, which makes this a big problem.
Comment #24
pfrenssenIf Firefox shouldn't reuse the user input on soft reload that is something that should be discussed on Bugzilla and fixed in Firefox itself. There are also addons available for Firefox that enforce hard refreshes for the people that like this functionality.
We shouldn't alter default behaviour of a browser. There are millions of people using Firefox, do we really want to dictate that for all of them their chosen browser's behaviour is "wrong" and we should "fix" it?
Comment #25
hchonov@pfrenssen, said like this it makes perfect sense to not change it and yes I agree you are right.
However if you have a medical application you would like to ensure that no mixed input might be caused if a user reloads the page in order to throw away all changes and then continues editing without noticing that the previous changes are still there. For one of our products the solution was to disable autocomplete at the form level.
+1 on the current patch.
Comment #26
alexpottIt's a shame that this is essentially untestable as we're working around a browser specific bug. I'm committing to 8.4.x for the moment just in case there are issues we've not identified yet.
Committed a83586c and pushed to 8.4.x. Thanks!
Comment #30
pfrenssenThis was marked as to be ported with the intention to backport it to 8.3.x after some incubation time in 8.4.x. Since 8.3.x is in the meantime unsupported this is not needed anymore. This can be marked as fixed now.