Closed Bug 1752250 Opened 3 years ago Closed 3 years ago

hidden input value refers to previous load value after location.reload()

Categories

(Core :: DOM: Forms, defect)

Firefox 96
defect

Tracking

()

RESOLVED FIXED
100 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox98 --- wontfix
firefox99 --- wontfix
firefox100 --- fixed

People

(Reporter: alvarso, Assigned: edgar)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(4 files)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:96.0) Gecko/20100101 Firefox/96.0

Steps to reproduce:

I have a website that uses "location.reload()" to go through a sequence of pages based on user input. The location always remains the same.

I can provide access to said website if its needed (I need to create an account for you, I'm OK doing that; note the site is in Spanish, but I'm happy to provide any guidance needed).

I don't know how to reproduce it otherwise, but I am able to reproduce it by removing the patch described below (it happens even in "Private Windows").

Note: other people in my team reported this bug to me, it is reproducible in other systems using Firefox, but it is only causing problems in this specific part of my system.

Actual results:

On Firefox ONLY (not Chrome or Safari), I'm having an issue of the code in the "console" (and therefore in javascript) showing an old value of a previous random element, while when looking at the "page source" it is correct. I realize "view page source" 'reloads' the page, so it might not be the same code if the server responds differently, so I looked at the original XHR in the console:
response 200 OK
cache-control: no-store, no-cache, must-revalidate, post-check=0, pre-check=0
response: has the CORRECT code

In other words, the code in the "response" is NOT what is being used in the "Console" and with JS, and that is really messing up my website when using Firefox!

When I shift-reload the page manually, or I use location.reload(true) (which is not HTML standard, but which works for Firefox), then the console and the page source show the same values, and my javascript works correctly. Currently I had to add a custom script for my website that will do a "location.reload(true)" only for firefox, so that users are not getting errors.

This started a couple weeks ago (early 2022, I don't have an exact date, unfortunately). It definitely was not happening back in late 2021. We have not changed that code at all. The only thing that happened since then were Firefox updates.

Example:

View Page Source:
<form id='doc_ejer_lecturaForm1'>
<input type='hidden' name='doc_ejer_orden_lecc' value='2'>
</form>

Console/Inspector:
<form id='doc_ejer_lecturaForm1'>
<input type="hidden" name="doc_ejer_orden_lecc" value="552">
</form>

The '552' is in fact a value that is used in other parts of the program, in input elements (hidden) that are used before this page. However, my server correctly sends the 2 as seen in the page source - but the console (& therefore JS) is not updated and the 552 from previous pages remains there. Again, this did not use to be a problem before.

I have triple checked, there is no JS that changes that value (I even renamed the inputs, removed ID values, renamed the form, etc, without changing any other JS, to make sure I did not have code changing the value of that element - plus it does NOT happen in Chrome or Safari). Further, this is happening randomly, although it does look limited to "input type='hidden'" elements so far.

Expected results:

The Console/Inspetor (& JS) should show the same values as in the XHR "Response" when a 200 response is received. The "raw" code in "Response" and in the "Console" should be identical (when no JS changes the DOM, which is my case).

(I don't want to speculate much on why exactly this difference happens - but I know that if the JS used the source shown in "Page Source" my problem would be solved; why I need to use a "location.reload(true)" I am not 100% sure, although it points to some issue with caching).

The Bugbug bot thinks this bug should belong to the 'DevTools::Console' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Component: Untriaged → Console
Product: Firefox → DevTools
Flags: needinfo?(nchevobbe)

Hello Alvar,

If you can provide an access to the site, that would be great (I understand a bit of spanish, so it should be okay). If you want to communicate the account detail privately, you can do it in https://chat.mozilla.org (I'm nchevobbe there)

On Firefox ONLY (not Chrome or Safari), I'm having an issue of the code in the "console" (and therefore in javascript) showing an old value of a previous random element

I'm not sure I follow, when you talk about the "Console", do you mean the "Console" panel? It looks like it could be the "Inspector" panel, but I'm not sure.
Could you provide screenshot of what you're seeing, it might make it easier for us to help .

Also, since you're saying this is something new, it would be really helpful if you could use https://mozilla.github.io/mozregression/ to tell us when it started misbehaving.

In the end, it doesn't seem like it's a DevTools related issue, but maybe something that has to do with the Cache? Do you have the issue if you don't open DevTools?

Thanks!

Flags: needinfo?(nchevobbe) → needinfo?(alvarso)
Attached image image.png (deleted) —

This screenshot shows the issue I'm having in one single image:

  • The "Inspector" (which I believe is what JS uses for its values) is showing
    <input type="hidden" name="doc_ejer_orden_lecc" value="552">
    (2 lines above the selected element in the screen capture)

  • The "Response" from the server, logged in the "console" area, shows that the server replied with:
    <input type='hidden' name='doc_ejer_orden_lecc' value='2'>

Because JS is using the value 552 instead of the value 2, my application breaks - ONLY in Firefox (there is no JS that changes that value anywhere in my code).

If in the same page I do a SHIFT-Reload (ie, no cache), then the "Inspector" (and JS) has the correct value for my hidden element (2), and my application works.

It does appear to me that this is a CACHE issue, yes - somehow the cache is remembering the value of previous hidden element (named differently) that did in fact have the value 552 before, and assigning it to this other hidden value.

I do want to point out, again, that the server response headers include:
cache-control no-store, no-cache, must-revalidate, post-check=0, pre-check=0

Flags: needinfo?(alvarso)

PS, I do have the issue if I do not open DevTools - agree, again, this is very likely not a DevTools issue, its very like a cache issue. However, it is by using DevTools that I can pin-point where the problem is, that's why I mentioned it. I'm happy to change the subject --- I just did not want to make a one-sided determination that it was a cache issue - I wanted at least feedback from someone in Mozilla. I'm happy to work with anyone in your team to solve this if it needs to be moved.

PPS, I'll try to do something with mozregresion, but that might take me a few days to do, since there's other things in the burners. Using 'location.reload(true)' is solving the problem for now, but I don't like that it increases bandwidth usage and slows down the user by having to reload everything. I'll put time with this as soon as I can, but that might be until next week.

Thanks Alvar for the additional information, the screenshot is helpful.

Since the issue is in an input, I wonder if this isn't about form autocomplete.
Could you try to add a autocomplete="off" attribute in your <form> to check if it's this?

In the meantime, I'm moving this to the form component so the team can investigate the issue.

Component: Console → DOM: Forms
Product: DevTools → Core
Summary: Console is showing different code than "Response" after location.reload() → hidden input value refers to previous load value after location.reload()

WOW, that's why I wanted someone in your team to know more about these things to give input.

YES, when I add autocomplete='off' to the form the error does not happen. When using a different page (than the example I gave you), where I did not add auto off, then the error happens.

However, I do want to point out that this is an exercise in finding where the BUG is, this is not correct behavior: my forms do not have any actual input fields, there is nothing to autocomplete for the user - instead, Firefox (and only Firefox, not Chrome or Safari) is 'autocompleting hidden fields', which does not make any sense in any way (both technical or logical - even in a form with actual user input fields, hidden fields should always be ignored by autocomplete!

Also note that this is not an effective "solution" to this problem, because in addition to being undesired behavior, it would mean I have to change every one of my forms with hidden elements, just for Firefox to work correctly.

So the good news is that I think you have narrowed it down better from maybe a "cache" issue to an "autocomplete" issue. Hopefully this can be tracked down and debugged easier knowing where it is.

PS, I did send you instructions in the chat on how to reproduce the problem (I took out the autocomplete again for now, so that the problem is easy to reproduce; let me know if you need me to send those instructions again)

PPS, I'll let you know next week when I have a chance to use mozregression

PPS, THANK YOU for helping me look into this, I really want our platform to keep using Firefox successfully!

I was able to reproduce on Firefox 91 using the following steps:

Steps to reproduce

  1. Go to data:text/html,<meta charset=utf8><form><input type=hidden name="test" value="original"></form><button id=show>show hidden value</button><button id=update>Update value</button><script>document.getElementById("show").addEventListener("click", () => {alert(document.querySelector("input").value)});document.getElementById("update").addEventListener("click", () => {document.querySelector("input").value = "updated"})</script>
  2. Click "show hidden value" , it should open an alert with original
  3. Click "update value", and "show hidden value", it should open an alert with updated
  4. soft-Reload the page
  5. Click "show hidden value" again

Actual results

it opens an alert with updated (so the value persisted across the reload)
I checked the same STR on latest Chrome and there the value does reset to original after reloading.

Nicolas,
Thank you for finding a way to reproduce in a somewhat easier than what I had.
I would like to note one thing: when I do a "soft reload" the name of the field changes, it is being assigned the value of a differently named object from the previous load. In my specific example the hidden input name='doc_ejer_orden_lecc' is being assigned the value of a previous hidden input name='id_leccion' (and the same happens for other differently named objects in other transitions of my SW). So it is not only that a hidden value is persisting, but that its being assigned based on something else than its identifier name=, and that really needs to be fixed!

The severity field is not set for this bug.
:peterv, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(peterv)

Hello,
My team was able to use Moz Regression to identify exactly when this bug was introduced:

  • Release 97 (2022-01-10) has the bug
  • Release 96 (2021-12-06) does not have the bug

This is really preventing us from having our customers use Firefox. I hope someone in the auto-complete team can look at it and fix it soon, because right now we have to tell our customers that they cannot use Firefox and must point them somewhere else, which I really hate to do.

I look forward to hearing from this team again, hopefully soon. (I hope you were not just waiting for me to use MozRegression, but now that its done, hopefully someone can fix the bug faster).

Thank you!

A regression range would be really useful here. I mean regression range using nightly builds.

QA Whiteboard: [qa-regression-triage]
Attached file MozRegression "bisecting" results (deleted) —

My team did a more detailed regression to include daily builds. Attached are the results. The summary is that the nighly build on 2021-12-13 was good, but the one on 2021-12-14 was bad.

Its also interesting to note that on the build of 2021-12-27 things were good again for a day, but then returned to error.

Hopefully the included details help find where the bug was introduced.

Thank you!

Reproduced on the latest Firefox versions on macOS Big Sur 11.6 and tried to look for a more accurate regression range using the STR from Comment 8 - but I can reproduce the issue all along to Firefox 70.0.1 (the older builds than that are not loading any content on my machine).

Nicolas, is there something else that I should take into consideration when replicating this issue?

Flags: needinfo?(nchevobbe)

(In reply to Simona Badau from comment #14)

Reproduced on the latest Firefox versions on macOS Big Sur 11.6 and tried to look for a more accurate regression range using the STR from Comment 8 - but I can reproduce the issue all along to Firefox 70.0.1 (the older builds than that are not loading any content on my machine).

Nicolas, is there something else that I should take into consideration when replicating this issue?

Not sure, but note that the STR of the reporter is slightly different (see https://bugzilla.mozilla.org/show_bug.cgi?id=1752250#c9)

Alvar, could we have access to that page, or something that would do something similar so we can better check what's happening?

Flags: needinfo?(nchevobbe) → needinfo?(alvarso)

Hello, these directions helped Nicolas before (I sent them via chat), they're still valid:

I have set our test website back to use location.reload() (without reload(true)), so that the problem can be replicated at: https://prueba.lecturainteligente.com.mx/escuela/li
After you go there, the address will be rewritten to https://prueba.lecturainteligente.com.mx/lector/index.php (by apache), and you should see a space to type in a password:

  • Clave: @nchevobbe - can you please share this pwd with your teammates? I sent it in the original message via 'chat.mozzilla.org'

Then select:

  • Grupo: Alvar - Estilo 2018
  • Alumno: Nom A Pate A Mate A
  • Password: 123
  • Click Entrar

(Yes, its a terrible password, but its only for accessing testing accounts, they don't give access to anything else ;-)
Click on Continuar (or wait a few seconds for it to be dismissed)
Click on Leccion 1 then on Entrar a Leccion

To reproduce:

  • Right click and inspect the button Repetir for the first excercise 1 - EC Ejercicio de Comprension (EC) (only this exercise has the problem in this page; the problem appears in other parts of the site too, but this one is very repeatable).
  • Above the button definition you will see the hidden value for:
    <input type="hidden" name="doc_ejer_orden_lecc" value="552">
  • Reload the page (without shift, don't clear cache) and then back in developer tools go to the original XHR request (scroll to the top of console) GET https://prueba.lecturainteligente.com.mx/lector/index.php and you'll see the 200 OK. When you look in the Response and select Raw look for id doc_ejer_lecturaForm1 and just below it is the correct server value
    <input type="hidden" name="doc_ejer_orden_lecc" value="2">

If you do a normal reload (without shift to clear cache), you will see this all the time. If you do a shift reload (clearing cache), you will see that the inspector correctly shows value="2".

Flags: needinfo?(alvarso)

According to Comment 13, the regression range was narrowed to:

Found commit message:
Bug 1732358 - Part 5: Add the fission rollout slug to the GRADUATION_SET, r=mythmon

Setting this to NEW, as the issue is reproducible following the steps from Comment 8.

Severity: -- → S3
Status: UNCONFIRMED → NEW
Has Regression Range: --- → yes
Ever confirmed: true
Regressed by: 1732358

(In reply to Simona Badau from comment #17)

According to Comment 13, the regression range was narrowed to:

Found commit message:
Bug 1732358 - Part 5: Add the fission rollout slug to the GRADUATION_SET, r=mythmon

Setting this to NEW, as the issue is reproducible following the steps from Comment 8.

Thanks for getting this commit. Unfortunately, it's not that helpful to narrow down the root cause. Flipping the fission preference on implies a big architecture change.

Flags: needinfo?(echen)

I reviewed the "See Also" bugs that Alice0775 linked, and I fully agree with 520561, and the title Autocomplete is too aggressive and overwrites values of hidden fields makes a lot of sense. I reviewed that bug report, and I do believe the symptoms indicated there are the same as what is happening again (that bug is from 13 years ago, it seems that some part of Autocomplete was re-activated from then).

Is there anything else I can give the FF team to try to fix this bug? I really would like to support FF again with my application.

It's more like a SHIP issue and not a recent regression as I could reproduce on 85 with fission enabled.
It seems like we somehow restore the form state after a location.reload() which loads a new page.

After comparing the SHIP and non-SHIP flow, I think the issue is around how we handle "SaveLayoutState" flag

Assignee: nobody → echen
Flags: needinfo?(peterv)
Flags: needinfo?(echen)

Thank you to all looking into this. I want to point out one thing that is very important from my end, and I think should be very concerning in general:

  • auto complete is assigning a value to an element with a different name/id

From my Comment 9:

  • Load 1: <input type="hidden" name="doc_ejer_orden_lecc" value="552">
  • location.reload()
  • Load 2: <input type="hidden" name="id_leccion" value="2"> get assigned value="552" by auto-complete

That means that the "SHIP" system is not even evaluating the names of elements to assign values, it has some sort of 'template' that overwrites values, regardless of the name. This is really scary to me. In the example I provide, there are many elements with name="id_leccion", but only the first one is overwritten by auto-complete. Therefore, its really ignoring code, and that seems like a very very bad idea.

Regardless of whether there is a "no-store" header, Firefox should either show the previous HTML with cached values, or show the new HTML without cached values. Don't show the new HTML with the previous values.

Please check that this patch also fixes the example in Bug 1760911 (marked as a duplicate).
https://bugzilla.mozilla.org/show_bug.cgi?id=1760911

Reproduce:

  1. visit https://gamma.bakacafe.com/sandbox/checkboxes.php
  2. check the 10th box and click "submit"
  3. refresh the page.

The 10th box moves to the top and has checked="checked" (which is intended). The 9th box also shows checked, but that input does NOT have checked="checked". This element has a different ID and a different value. Regardless of whether there is a "no-store" header, the 9th box should never show as checked.

(In reply to Frank Forte from comment #27)

The 10th box moves to the top and has checked="checked" (which is intended). The 9th box also shows checked, but that input does NOT have checked="checked". This element has a different ID and a different value. Regardless of whether there is a "no-store" header, the 9th box should never show as checked.

This is how Gecko currently behave on form restoration, i.e. base on order.
There is a bug for that, see bug 539228, We could move the discussion there.

(In reply to Edgar Chen [:edgar] from comment #28)

(In reply to Frank Forte from comment #27)

The 10th box moves to the top and has checked="checked" (which is intended). The 9th box also shows checked, but that input does NOT have checked="checked". This element has a different ID and a different value. Regardless of whether there is a "no-store" header, the 9th box should never show as checked.

This is how Gecko currently behave on form restoration, i.e. base on order.
There is a bug for that, see bug 539228, We could move the discussion there.

I'm OK joining different bugs, especially if it gets us to a faster resolution. I do believe that 539228 addresses one possible solution, and likely the best one, because on re-load forms must consider the name/id of an element, the order is not sufficient (and in my opinion an error/bug that in any correctly programmed page would not affect users). At the same time, if the page states 'no-store' then the cached form should not be loaded at all (in order or otherwise), which is another bug (although one that may affect users and I could see why a user may want a reload to keep form values if nothing else changes).

So I leave it to the FF team to decide if its best to join 'discussions', or if this can be a separate fix based on 'no-store' settings. Let me know if I can help to move this closer to resolution (given that I do not know the gecko source code).

(In reply to Edgar Chen [:edgar] from comment #28)

(In reply to Frank Forte from comment #27)

The 10th box moves to the top and has checked="checked" (which is intended). The 9th box also shows checked, but that input does NOT have checked="checked". This element has a different ID and a different value. Regardless of whether there is a "no-store" header, the 9th box should never show as checked.

This is how Gecko currently behave on form restoration, i.e. base on order.
There is a bug for that, see bug 539228, We could move the discussion there.

I think both bugs can be solved by the same approach, so maybe linking that bug to this more recent discussion will help (that bug is 12 years old).

I am concerned that a no-store header approach is not enough. Sometimes a server might return new HTML even though the page was allowed to be cached on a previous request. I believe in this case Firefox might show the cached HTML, then update it to the new HTML once the HTTP request is complete.

The following approach could fix BOTH bugs (and probably a few more):

If Firefox is rendering new HTML from the server, show the new HTML without cached values.
If the page is cached, show the previous HTML with cached values.

I recommend no mixing and matching. As soon as Firefox uses new HTML from the server (even if it is right after rendering the cached page), then it should NOT populate the cached values, or re-populate them based on the updated HTML. Otherwise it could clobber new values like CSRF tokens in hidden inputs, check boxes, etc.

This approach means that web developers who don't know how to set cache headers will still be safe from either bug (both are the same issue, where Firefox populates inputs with the wrong values or checked property).

Pushed by echen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8d40bb06b8e4 Part 1: Refactor form restoration test; r=peterv https://hg.mozilla.org/integration/autoland/rev/e881ef466324 Part 2: [SHIP] Don't restore form data when reload a page with a no-store header; r=peterv
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 100 Branch

My team confirmed that the nightly build with the fix DOES correct our problem (we do send the no-store header).

Ultimately bug 539228 would be an even better fix, but we can leave that separately (since it seems that its very low priority on the queue at this point).

So I agree that this bug can be closed, but the other one should remain open.

Thank you :-)

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: