Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-21720

Don't ask for confirmation when it doesn't make any sense

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Major Major
    • core
    • 1.538+

      Introduced in this commit:

      https://github.com/jenkinsci/jenkins/pull/962/files#r9552059

      A few of the odd behaviors:

      • Asks for confirmation when browsing away after navigating between form elements by pressing tab
      • Asks for confirmation when triggering form submit using keyboard
      • Asks for confirmation when navigating away after clicking an Advanced… button.

          [JENKINS-21720] Don't ask for confirmation when it doesn't make any sense

          Daniel Beck added a comment -

          Daniel Beck added a comment - https://github.com/jenkinsci/jenkins/pull/1175

          Code changed in jenkins
          User: Daniel Beck
          Path:
          core/src/main/resources/hudson/logging/LogRecorder/configure.jelly
          core/src/main/resources/hudson/model/Computer/configure.jelly
          core/src/main/resources/hudson/model/ComputerSet/_new.jelly
          core/src/main/resources/hudson/model/View/configure.jelly
          core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy
          core/src/main/resources/jenkins/model/Jenkins/configure.jelly
          core/src/main/resources/lib/form/confirm.js
          http://jenkins-ci.org/commit/jenkins/d85b86e71921f3a43a1c49701f66a1ba2b23477d
          Log:
          [FIXED JENKINS-21720] Improved config confirmation.

          Ask for confirmation when navigating away from node, global,
          security, view and log configuration after changes.

          Don't require confirmation when pressing keys without changing
          (e.g. pressing Tab for navigation) and when pressing Advanced
          buttons.

          Don't require confirmation when submitting the form.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Daniel Beck Path: core/src/main/resources/hudson/logging/LogRecorder/configure.jelly core/src/main/resources/hudson/model/Computer/configure.jelly core/src/main/resources/hudson/model/ComputerSet/_new.jelly core/src/main/resources/hudson/model/View/configure.jelly core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy core/src/main/resources/jenkins/model/Jenkins/configure.jelly core/src/main/resources/lib/form/confirm.js http://jenkins-ci.org/commit/jenkins/d85b86e71921f3a43a1c49701f66a1ba2b23477d Log: [FIXED JENKINS-21720] Improved config confirmation. Ask for confirmation when navigating away from node, global, security, view and log configuration after changes. Don't require confirmation when pressing keys without changing (e.g. pressing Tab for navigation) and when pressing Advanced buttons. Don't require confirmation when submitting the form.

          Code changed in jenkins
          User: Daniel Beck
          Path:
          core/src/main/resources/hudson/model/ComputerSet/configure.jelly
          core/src/main/resources/hudson/model/Run/configure.jelly
          core/src/main/resources/hudson/model/User/configure.jelly
          core/src/main/resources/hudson/model/labels/LabelAtom/configure.jelly
          http://jenkins-ci.org/commit/jenkins/cd0fae16f00805e63832c4009a4d6984622327c0
          Log:
          JENKINS-21720 Added User, Run, ComputerSet, and LabelAtom

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Daniel Beck Path: core/src/main/resources/hudson/model/ComputerSet/configure.jelly core/src/main/resources/hudson/model/Run/configure.jelly core/src/main/resources/hudson/model/User/configure.jelly core/src/main/resources/hudson/model/labels/LabelAtom/configure.jelly http://jenkins-ci.org/commit/jenkins/cd0fae16f00805e63832c4009a4d6984622327c0 Log: JENKINS-21720 Added User, Run, ComputerSet, and LabelAtom

          Code changed in jenkins
          User: Oliver Gondža
          Path:
          core/src/main/resources/hudson/logging/LogRecorder/configure.jelly
          core/src/main/resources/hudson/model/Computer/configure.jelly
          core/src/main/resources/hudson/model/ComputerSet/_new.jelly
          core/src/main/resources/hudson/model/ComputerSet/configure.jelly
          core/src/main/resources/hudson/model/Job/configure.jelly
          core/src/main/resources/hudson/model/Run/configure.jelly
          core/src/main/resources/hudson/model/User/configure.jelly
          core/src/main/resources/hudson/model/View/configure.jelly
          core/src/main/resources/hudson/model/labels/LabelAtom/configure.jelly
          core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy
          core/src/main/resources/jenkins/model/Jenkins/configure.jelly
          core/src/main/resources/lib/form/confirm.js
          http://jenkins-ci.org/commit/jenkins/fc471153db957be8d06d2fea63306e8b2f060462
          Log:
          Merge pull request #1175 from daniel-beck/JENKINS-21720-2

          [FIXED JENKINS-20597 JENKINS-21720] Improved config confirmation.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oliver Gondža Path: core/src/main/resources/hudson/logging/LogRecorder/configure.jelly core/src/main/resources/hudson/model/Computer/configure.jelly core/src/main/resources/hudson/model/ComputerSet/_new.jelly core/src/main/resources/hudson/model/ComputerSet/configure.jelly core/src/main/resources/hudson/model/Job/configure.jelly core/src/main/resources/hudson/model/Run/configure.jelly core/src/main/resources/hudson/model/User/configure.jelly core/src/main/resources/hudson/model/View/configure.jelly core/src/main/resources/hudson/model/labels/LabelAtom/configure.jelly core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy core/src/main/resources/jenkins/model/Jenkins/configure.jelly core/src/main/resources/lib/form/confirm.js http://jenkins-ci.org/commit/jenkins/fc471153db957be8d06d2fea63306e8b2f060462 Log: Merge pull request #1175 from daniel-beck/ JENKINS-21720 -2 [FIXED JENKINS-20597 JENKINS-21720] Improved config confirmation.

          dogfood added a comment -

          Integrated in jenkins_main_trunk #3321
          [FIXED JENKINS-21720] Improved config confirmation. (Revision d85b86e71921f3a43a1c49701f66a1ba2b23477d)
          JENKINS-21720 Added User, Run, ComputerSet, and LabelAtom (Revision cd0fae16f00805e63832c4009a4d6984622327c0)

          Result = SUCCESS
          daniel-beck : d85b86e71921f3a43a1c49701f66a1ba2b23477d
          Files :

          • core/src/main/resources/hudson/model/View/configure.jelly
          • core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy
          • core/src/main/resources/hudson/model/ComputerSet/_new.jelly
          • core/src/main/resources/hudson/logging/LogRecorder/configure.jelly
          • core/src/main/resources/jenkins/model/Jenkins/configure.jelly
          • core/src/main/resources/lib/form/confirm.js
          • core/src/main/resources/hudson/model/Computer/configure.jelly

          daniel-beck : cd0fae16f00805e63832c4009a4d6984622327c0
          Files :

          • core/src/main/resources/hudson/model/labels/LabelAtom/configure.jelly
          • core/src/main/resources/hudson/model/User/configure.jelly
          • core/src/main/resources/hudson/model/ComputerSet/configure.jelly
          • core/src/main/resources/hudson/model/Run/configure.jelly

          dogfood added a comment - Integrated in jenkins_main_trunk #3321 [FIXED JENKINS-21720] Improved config confirmation. (Revision d85b86e71921f3a43a1c49701f66a1ba2b23477d) JENKINS-21720 Added User, Run, ComputerSet, and LabelAtom (Revision cd0fae16f00805e63832c4009a4d6984622327c0) Result = SUCCESS daniel-beck : d85b86e71921f3a43a1c49701f66a1ba2b23477d Files : core/src/main/resources/hudson/model/View/configure.jelly core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy core/src/main/resources/hudson/model/ComputerSet/_new.jelly core/src/main/resources/hudson/logging/LogRecorder/configure.jelly core/src/main/resources/jenkins/model/Jenkins/configure.jelly core/src/main/resources/lib/form/confirm.js core/src/main/resources/hudson/model/Computer/configure.jelly daniel-beck : cd0fae16f00805e63832c4009a4d6984622327c0 Files : core/src/main/resources/hudson/model/labels/LabelAtom/configure.jelly core/src/main/resources/hudson/model/User/configure.jelly core/src/main/resources/hudson/model/ComputerSet/configure.jelly core/src/main/resources/hudson/model/Run/configure.jelly

          Code changed in jenkins
          User: Daniel Beck
          Path:
          core/src/main/resources/hudson/model/ComputerSet/configure.jelly
          core/src/main/resources/hudson/model/User/configure.jelly
          core/src/main/resources/hudson/model/labels/LabelAtom/configure.jelly
          http://jenkins-ci.org/commit/jenkins/8057dd355c122f948a58e0db1804d4b538f612c4
          Log:
          JENKINS-21720 Added User, Run, ComputerSet, and LabelAtom

          (cherry picked from commit cd0fae16f00805e63832c4009a4d6984622327c0)

          Conflicts:
          core/src/main/resources/hudson/model/Run/configure.jelly

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Daniel Beck Path: core/src/main/resources/hudson/model/ComputerSet/configure.jelly core/src/main/resources/hudson/model/User/configure.jelly core/src/main/resources/hudson/model/labels/LabelAtom/configure.jelly http://jenkins-ci.org/commit/jenkins/8057dd355c122f948a58e0db1804d4b538f612c4 Log: JENKINS-21720 Added User, Run, ComputerSet, and LabelAtom (cherry picked from commit cd0fae16f00805e63832c4009a4d6984622327c0) Conflicts: core/src/main/resources/hudson/model/Run/configure.jelly

          Code changed in jenkins
          User: Daniel Beck
          Path:
          core/src/main/resources/hudson/logging/LogRecorder/configure.jelly
          core/src/main/resources/hudson/model/Computer/configure.jelly
          core/src/main/resources/hudson/model/ComputerSet/_new.jelly
          core/src/main/resources/hudson/model/View/configure.jelly
          core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy
          core/src/main/resources/jenkins/model/Jenkins/configure.jelly
          core/src/main/resources/lib/form/confirm.js
          http://jenkins-ci.org/commit/jenkins/85f628fec320a6bdf41f3aeaa43a40ff3a8b5ab9
          Log:
          [FIXED JENKINS-21720] Improved config confirmation.

          Ask for confirmation when navigating away from node, global,
          security, view and log configuration after changes.

          Don't require confirmation when pressing keys without changing
          (e.g. pressing Tab for navigation) and when pressing Advanced
          buttons.

          Don't require confirmation when submitting the form.

          (cherry picked from commit d85b86e71921f3a43a1c49701f66a1ba2b23477d)

          Compare: https://github.com/jenkinsci/jenkins/compare/b05d9938c2f6...85f628fec320

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Daniel Beck Path: core/src/main/resources/hudson/logging/LogRecorder/configure.jelly core/src/main/resources/hudson/model/Computer/configure.jelly core/src/main/resources/hudson/model/ComputerSet/_new.jelly core/src/main/resources/hudson/model/View/configure.jelly core/src/main/resources/hudson/security/GlobalSecurityConfiguration/index.groovy core/src/main/resources/jenkins/model/Jenkins/configure.jelly core/src/main/resources/lib/form/confirm.js http://jenkins-ci.org/commit/jenkins/85f628fec320a6bdf41f3aeaa43a40ff3a8b5ab9 Log: [FIXED JENKINS-21720] Improved config confirmation. Ask for confirmation when navigating away from node, global, security, view and log configuration after changes. Don't require confirmation when pressing keys without changing (e.g. pressing Tab for navigation) and when pressing Advanced buttons. Don't require confirmation when submitting the form. (cherry picked from commit d85b86e71921f3a43a1c49701f66a1ba2b23477d) Compare: https://github.com/jenkinsci/jenkins/compare/b05d9938c2f6...85f628fec320

          This issue is still there and it comes up under high CPU loads, which makes some HTTP request longer than 100 milliseconds.

          Antonio Muñiz added a comment - This issue is still there and it comes up under high CPU loads, which makes some HTTP request longer than 100 milliseconds.

          Antonio Muñiz added a comment - PR sent: https://github.com/jenkinsci/jenkins/pull/1854

          Code changed in jenkins
          User: tfennelly
          Path:
          .gitignore
          changelog.html
          cli/pom.xml
          core/pom.xml
          core/src/main/java/hudson/model/AbstractProject.java
          core/src/main/java/hudson/model/Run.java
          core/src/main/resources/hudson/widgets/HistoryWidget/index.jelly
          core/src/main/resources/lib/form/confirm.js
          core/src/main/resources/lib/form/select/select.js
          core/src/main/resources/lib/layout/layout.jelly
          plugins/pom.xml
          pom.xml
          test/pom.xml
          test/src/test/groovy/hudson/model/AbstractProjectTest.groovy
          war/pom.xml
          http://jenkins-ci.org/commit/jenkins/95ca3da67d217c90d31819ec92e521e2072acd5a
          Log:
          Merge branch 'master' into plugin-manager-dependants

          • master:
            Update the changelog by new merges:
            [FIXED JENKINS-30569] HistoryWidget: fix JS syntax error
            [FIXED JENKINS-29014] render API link conditional on getApi() presence
            JENKINS-21720 JS alert preventig to leave a configuration page even without formulary changes
            add ctags file 'tags' to .gitignore
            [maven-release-plugin] prepare release jenkins-1.632
            [FIXED JENKINS-29888] Handling all exceptions returned by logRotator
            JENKINS-30742 Fixed possible NPE in AbstractProject.resolveForCLI()

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: tfennelly Path: .gitignore changelog.html cli/pom.xml core/pom.xml core/src/main/java/hudson/model/AbstractProject.java core/src/main/java/hudson/model/Run.java core/src/main/resources/hudson/widgets/HistoryWidget/index.jelly core/src/main/resources/lib/form/confirm.js core/src/main/resources/lib/form/select/select.js core/src/main/resources/lib/layout/layout.jelly plugins/pom.xml pom.xml test/pom.xml test/src/test/groovy/hudson/model/AbstractProjectTest.groovy war/pom.xml http://jenkins-ci.org/commit/jenkins/95ca3da67d217c90d31819ec92e521e2072acd5a Log: Merge branch 'master' into plugin-manager-dependants master: Update the changelog by new merges: [FIXED JENKINS-30569] HistoryWidget: fix JS syntax error [FIXED JENKINS-29014] render API link conditional on getApi() presence JENKINS-21720 JS alert preventig to leave a configuration page even without formulary changes add ctags file 'tags' to .gitignore [maven-release-plugin] prepare release jenkins-1.632 [FIXED JENKINS-29888] Handling all exceptions returned by logRotator JENKINS-30742 Fixed possible NPE in AbstractProject.resolveForCLI()

          Code changed in jenkins
          User: Antonio Muñiz
          Path:
          core/src/main/resources/lib/form/confirm.js
          core/src/main/resources/lib/form/select/select.js
          http://jenkins-ci.org/commit/jenkins/f63407a87e9dc1c1f79c9a7d885cdd58a55ed657
          Log:
          JENKINS-21720 JS alert preventig to leave a configuration page even without formulary changes

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Antonio Muñiz Path: core/src/main/resources/lib/form/confirm.js core/src/main/resources/lib/form/select/select.js http://jenkins-ci.org/commit/jenkins/f63407a87e9dc1c1f79c9a7d885cdd58a55ed657 Log: JENKINS-21720 JS alert preventig to leave a configuration page even without formulary changes

          Code changed in jenkins
          User: Oleg Nenashev
          Path:
          core/src/main/resources/lib/form/confirm.js
          core/src/main/resources/lib/form/select/select.js
          http://jenkins-ci.org/commit/jenkins/42312cc3ac90f130394b9aa64ab3ec316f08097f
          Log:
          Merge pull request #1854 from amuniz/JENKINS-21720

          JENKINS-21720 JS alert preventing to leave a configuration page without changes

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oleg Nenashev Path: core/src/main/resources/lib/form/confirm.js core/src/main/resources/lib/form/select/select.js http://jenkins-ci.org/commit/jenkins/42312cc3ac90f130394b9aa64ab3ec316f08097f Log: Merge pull request #1854 from amuniz/ JENKINS-21720 JENKINS-21720 JS alert preventing to leave a configuration page without changes

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oleg Nenashev Path: changelog.html http://jenkins-ci.org/commit/jenkins/a79e2f144a932fe0a97b524f79133d1ea5d0fef0 Log: Update the changelog by new merges: JENKINS-29888 - https://github.com/jenkinsci/jenkins/pull/1790 JENKINS-30742 - https://github.com/jenkinsci/jenkins/pull/1849 JENKINS-29014 - https://github.com/jenkinsci/jenkins/pull/1850 JENKINS-21720 - https://github.com/jenkinsci/jenkins/pull/1854 JENKINS-30569 - https://github.com/jenkinsci/jenkins/pull/1856

          Jesse Glick added a comment -

          Is this supposed to be considered fixed now?

          Jesse Glick added a comment - Is this supposed to be considered fixed now?

          Yes, it's fixed. I thought it was automatically closed by the bot.

          Antonio Muñiz added a comment - Yes, it's fixed. I thought it was automatically closed by the bot.

          Code changed in jenkins
          User: Antonio Muñiz
          Path:
          core/src/main/resources/lib/form/confirm.js
          core/src/main/resources/lib/form/select/select.js
          http://jenkins-ci.org/commit/jenkins/acd341d5db724bc49fdbc52220f9e938813f5663
          Log:
          JENKINS-21720 JS alert preventig to leave a configuration page even without formulary changes

          (cherry picked from commit f63407a87e9dc1c1f79c9a7d885cdd58a55ed657)

          Compare: https://github.com/jenkinsci/jenkins/compare/012071befcc2...acd341d5db72

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Antonio Muñiz Path: core/src/main/resources/lib/form/confirm.js core/src/main/resources/lib/form/select/select.js http://jenkins-ci.org/commit/jenkins/acd341d5db724bc49fdbc52220f9e938813f5663 Log: JENKINS-21720 JS alert preventig to leave a configuration page even without formulary changes (cherry picked from commit f63407a87e9dc1c1f79c9a7d885cdd58a55ed657) Compare: https://github.com/jenkinsci/jenkins/compare/012071befcc2...acd341d5db72

          dogfood added a comment -

          Integrated in jenkins_main_trunk #4358
          JENKINS-21720 JS alert preventig to leave a configuration page even without formulary changes (Revision acd341d5db724bc49fdbc52220f9e938813f5663)

          Result = UNSTABLE
          ogondza : acd341d5db724bc49fdbc52220f9e938813f5663
          Files :

          • core/src/main/resources/lib/form/select/select.js
          • core/src/main/resources/lib/form/confirm.js

          dogfood added a comment - Integrated in jenkins_main_trunk #4358 JENKINS-21720 JS alert preventig to leave a configuration page even without formulary changes (Revision acd341d5db724bc49fdbc52220f9e938813f5663) Result = UNSTABLE ogondza : acd341d5db724bc49fdbc52220f9e938813f5663 Files : core/src/main/resources/lib/form/select/select.js core/src/main/resources/lib/form/confirm.js

          Andrei Tuicu added a comment -

          Hello, amuniz,

          This is just my opinion, but I think that it would be better to allow change events when a select is filled for the first time. For example, if you have a select on which other selects depend; in this case, when the main select is filled, I want my doFill${Field} methods to be called for the dependencies so they are filled based on the value of said main select. In the end, there is an actual change, because that particular select changes from nothing to the first value.

          What do you think? In my opinion we shouldn't have this check here:
          https://github.com/jenkinsci/jenkins/blob/master/core/src/main/resources/lib/form/select/select.js#L51

          I'm asking, because it is causing bugs in a plugin that I'm developing. I was relying on that change event.

          Thank you,
          Andrei

          Andrei Tuicu added a comment - Hello, amuniz , This is just my opinion, but I think that it would be better to allow change events when a select is filled for the first time. For example, if you have a select on which other selects depend; in this case, when the main select is filled, I want my doFill${Field} methods to be called for the dependencies so they are filled based on the value of said main select. In the end, there is an actual change, because that particular select changes from nothing to the first value. What do you think? In my opinion we shouldn't have this check here: https://github.com/jenkinsci/jenkins/blob/master/core/src/main/resources/lib/form/select/select.js#L51 I'm asking, because it is causing bugs in a plugin that I'm developing. I was relying on that change event. Thank you, Andrei

          andreituicu The JS alert should be shown only when there was some change made by the user, otherwise we get confirmation alerts when there were no change!

          No user interaction must imply no navigation confirmation.

          Antonio Muñiz added a comment - andreituicu The JS alert should be shown only when there was some change made by the user, otherwise we get confirmation alerts when there were no change! No user interaction must imply no navigation confirmation.

            amuniz Antonio Muñiz
            danielbeck Daniel Beck
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved: