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

          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: