Regression caused by 1358025 (likely part 4) - Tab history is not available on ^Z. I'm not sure if I should or might not be able to ^Z into a different tab's history, but I can't even undo/^Z to pages in the same website domain anymore. Mozregression: INFO: Last good revision: 92c5898618c21d80d8cccd49bc08ec5fc8924a45 INFO: First bad revision: 029d3143f52d17a218cd7dcb33ec3003fe5a8d9d INFO: Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=92c5898618c21d80d8cccd49bc08ec5fc8924a45&tochange=029d3143f52d17a218cd7dcb33ec3003fe5a8d9d

Just to clarify, ^Z as in CTRL-Z / undo

(In reply to rctgamer3 from comment #0) > Regression caused by 1358025 (likely part 4) - Tab history is not available > on ^Z. I'm not sure if I should or might not be able to ^Z into a different > tab's history, but I can't even undo/^Z to pages in the same website domain > anymore. It should not be possible anymore to undo changes from a different tab. do you see a problem while staying in the current tab? (I'm not sure what you mean by "I can't even undo/^Z to pages in the same website domain anymore", if it's in the same tab or in a different one.

(In reply to Marco Bonardo [::mak] from comment #2) > It should not be possible anymore to undo changes from a different tab. > do you see a problem while staying in the current tab? (I'm not sure what > you mean by "I can't even undo/^Z to pages in the same website domain > anymore", if it's in the same tab or in a different one. Undo/history is unavailable while staying in the current tab.

Could you please provide Step to reproduce the issue in comment 3? Thank you.

Assuming these are the STR, I cannot reproduce on mac or win10, in latest 55 beta: 1) in awesomebar, start to type google.com 2) type cmd+z to see history of related domains in awesomebar

STR: 1) Type: "foobar" (without quotation marks) 2) Erase everything 3) Type: "barfoo" 4) Hit ^Z 5) foobar should show up when ^Z is performed, but it only deletes the word until the first character, e.g. only "b" shows up instead of the previously entered value. STR #2 (similar): 1) Navigate to mozilla.org 2) Navigate to publicsuffix.org 2) Naviate to mozillians.org 3) Hit ^Z 4) History, publicsuffix.org and mozilla.org do not show up when pressing ^Z. No history at all shows up. This worked fine before bug 1358025 was "fixed".

Makoto-san, what do you think about this? Sounds different from bug 1369592, since it doesn't involve different tabs.

(In reply to rctgamer3 from comment #6) > STR #2 (similar): > 1) Navigate to mozilla.org > 2) Navigate to publicsuffix.org > 2) Naviate to mozillians.org > 3) Hit ^Z > 4) History, publicsuffix.org and mozilla.org do not show up when pressing > ^Z. No history at all shows up. Since awesome bar doesn't use setUserInput, it is correct as textbox. Since STR1 seems to be another issue, I will look it.

STR #1 should be undo transaction. autocomplete.xml doesn't use setUserInput for value setter. So we should use it instead.

after updating test, I will resend review

Humm, since setUserInput moves focus and posts input event, some doorhanger tests are failure.

Makoto, any updates? Thanks!

according to comment #6, reporter reports 2 issues, (In reply to rctgamer3 from comment #6) > STR: > > 1) Type: "foobar" (without quotation marks) > 2) Erase everything > 3) Type: "barfoo" > 4) Hit ^Z > 5) foobar should show up when ^Z is performed, but it only deletes the word > until the first character, e.g. only "b" shows up instead of the previously > entered value. patch is coming for this. > STR #2 (similar): > 1) Navigate to mozilla.org > 2) Navigate to publicsuffix.org > 2) Naviate to mozillians.org > 3) Hit ^Z > 4) History, publicsuffix.org and mozilla.org do not show up when pressing > ^Z. No history at all shows up. :mak, is this behavior is expected as product design? Although I test on all major browsers (Safari, Chrome, and IE), undo operation doesn't back to previous address. So current behavior is same as all major browsers. All browser's text box doesn't create undo transaction by input.value setter.

(In reply to Makoto Kato [:m_kato] from comment #15) > > STR #2 (similar): > > 1) Navigate to mozilla.org > > 2) Navigate to publicsuffix.org > > 2) Naviate to mozillians.org > > 3) Hit ^Z > > 4) History, publicsuffix.org and mozilla.org do not show up when pressing > > ^Z. No history at all shows up. > > :mak, is this behavior is expected as product design? Although I test on > all major browsers (Safari, Chrome, and IE), undo operation doesn't back to > previous address. Maybe it's platform dependent? I tested Chrome and Edge on Windows 10, and both on ^Z suggested me the previous urls (obviously as far as I do all the load in the same tab). I just did paste+Enter of each of the above urls in a tab and then ^z enough times. I honestly don't have enough data to tell if this behavior is expected by users or not, but evidence seems to be that we don't behave like the other major browsers.

In the end, I think we should try to be coherent with the other browsers, in lack of better data to evaluate the case.

(In reply to Marco Bonardo [::mak] from comment #16) > Maybe it's platform dependent? I tested Chrome and Edge on Windows 10, and > both on ^Z suggested me the previous urls (obviously as far as I do all the > load in the same tab). I just did paste+Enter of each of the above urls in a > tab and then ^z enough times. I test on both macOS and Widnows. STR on Chrome for Windows 1. type url: https://www.mozilla.org/en-US/ and [Enter] 2. Click "Firefox" link, then navigate to https://www.mozilla.org/en_US/firefox 3. Click "Internet Health" link, then navigate to https://www.mozilla.org/en-US/internet-health/ 4. Ctrl+Z Result: url bar is https://www.mozilla.org When same steps on Firefox 54, url bar is https://www.mozilla.org/en_US/firefox. It isn't same behavior on each browser.

Of course, maybe, we can be back to same behavior (always turning on undo stack) on chrome process (but I don't know this is acceptable). If we move from XUL to HTML for UI, we will hit same issue. So I want to know correct design for address bar.

(In reply to Makoto Kato [:m_kato] from comment #18) > Result: > url bar is https://www.mozilla.org correct is https://www.mozilla.org/en-US/

(In reply to Makoto Kato [:m_kato] from comment #18) > STR on Chrome for Windows > 1. type url: https://www.mozilla.org/en-US/ and [Enter] > 2. Click "Firefox" link, then navigate to > https://www.mozilla.org/en_US/firefox > 3. Click "Internet Health" link, then navigate to > https://www.mozilla.org/en-US/internet-health/ > 4. Ctrl+Z Ok, what I tested was pasting and visiting 3 unrelated urls as suggested in comment 6, rather than browsing through links. I tried your steps in Edge, it's bogus. It seems to go back if I press CTRL+Z many times, but then it can't redo properly. It's just bogus. Chrome doesn't do anything. I don't think these specific steps are a big deal, it's not urls the user typed, so we likely don't care to provide undo here, I'm fine with it being broken. I think it's more important the case where the user types values. In the same tab: 1) paste/type mozilla.org into the urlbar, press Enter 2) paste/type publicsuffix.org, press Enter 3) paste/type mozillians.org, press Enter 4) Hit ^Z 5) History, publicsuffix.org and mozilla.org do not show up when pressing ^Z. These values have been typed by the user, and as such ^Z *should* probably cycle through them, and both Edge and Chrome cycle through them afaict. This is a bit "bogus" in all the browsers, so I don't think it has a critical priority. If there's an easy way to at least cycle through stuff the user typed/pasted, it would likely be preferred.

Could you please clarify which of the STR in comment 6 this is handling? Is it instead handling comment 5? By looking at the patch it seems like it's fixing a totally different bug with autoFill, that is not reported in this bug. that sounds like something to fix, but I cannot relate the reported failures with the patch. Looks like we have 3 issues reported here, 1 in comment 5 and 2 in comment 6, and the patch is just handling comment 5? I guess we may want to handle the cases in dependent bugs, one per issue, to simplify tracking. Regarding the patch, when the controller does autofill, it invokes setTextValueWithReason with aReason == Components.interfaces.nsIAutoCompleteInput.TEXTVALUE_REASON_COMPLETEDEFAULT. The textValue setter invokes the value setter, so we could set a temporary property to know that the value setter has been invoked as a consequence of an autoFill, without having to guess through .focused (provided that is the scope).

(In reply to Marco Bonardo [::mak] from comment #24) > Could you please clarify which of the STR in comment 6 this is handling? Is > it instead handling comment 5? > By looking at the patch it seems like it's fixing a totally different bug > with autoFill, that is not reported in this bug. that sounds like something > to fix, but I cannot relate the reported failures with the patch. This fixes part 1. case only. Since setUserInput moves focus, this method won't help STR #2's issue (like comment #18) > Looks like we have 3 issues reported here, 1 in comment 5 and 2 in comment > 6, and the patch is just handling comment 5? I guess we may want to handle > the cases in dependent bugs, one per issue, to simplify tracking. OK, I will separate bugs. > Regarding the patch, when the controller does autofill, it invokes > setTextValueWithReason with aReason == > Components.interfaces.nsIAutoCompleteInput.TEXTVALUE_REASON_COMPLETEDEFAULT. > The textValue setter invokes the value setter, so we could set a temporary > property to know that the value setter has been invoked as a consequence of > an autoFill, without having to guess through .focused (provided that is the > scope). OK, I want to detect auto fill. So I should use it if possible. Also, value setter is used on navigating URL (to update url address on address bar). But this setter to update current display URL doesn't want focus change, so I cannot use setUserInput for all situation.

Comment on attachment 8883519 [details] Bug 1376147 - Part 1. autocomplete widget should use setUserInput to allow undo transaction. Clearing review, I'd prefer this to be split across dependency bugs, and I gave some suggestions already. In case I'm away, you can ask for review to :adw Note that yesterday I've seen a discussion in #developers between Masayuki and Ehsan about the possibility to restore old edit history behavior for chrome context, so this may maybe become obsolete if that happens...

in particular, I think they were discussing the fix in bug 1386222 (I don't know the editor code, so you can tell better than me)

Marking as 56 fix-optional, given Comment #26. We may want to do the same for 57...

I can't recreate this bug on nightly (58) haven't tried others...

I think bug 1386222 has pretty much restored the behavior for xul as it was before, so as far as we don't move to html this bug may not reproduce.

P5 until we understand if this is still an issue, could be WFM.

ni on Andrei for QA to try to reproduce.

P5 would indicate that we're not working on a patch, but would accept one, so unsetting priority as this bug is still being triaged. If you're unable to get a reproduction, don't be shy about resolving as WFM.

Yeah, for now I'd resolve to WFM, because it looks like recent changes have reverted the behavior. Though, the behavior is not crystal clear and we may still have some edge-cases not working very well. We'll look into those bugs when identified and filed.