Fix semantic checks of milestones to get field values from form or appropriate stages (#3583)

* Reapplied all changes from #3552 to a new clone of the repo.

* Remove spurious import

* Update most screenshots.

* playwright tests working

* Ironing out details

* Some tweaks.

* Force tests to run one at a time.  slow but reliable

* Scroll to the next field after the shipped milestone, so we can see the warning.

* Only allow 1 worker, to avoid shared session problems.  Disable generating 'reporter' link after running tests.

* Check that a warning does show up.  Re-enable testing all platforms.

* Add test of feature page, adding an origin trial stage.

* Add an optional filename parameter to the pwtests-update command

* Add test and screenshot of OT milestones error.

* Scroll OT milestone fields into view.

* Mask history, and scroll OT panels into view.

* Maybe make edit page test more reliable.

* Call testInfo.setTimeout in test.beforeEach.

* Double the total timeout for playwright ci tests to 30 minutes.

* Add another few steps to the stage tests.

* Fix syntax error

* Update docs and comments.

* Update many messages.

* Update images for tests.

* Update client-src/elements/chromedash-form-field.js

Co-authored-by: Daniel Smith <56164590+DanielRyanSmith@users.noreply.github.com>

* Fix comments.

* Update screenshots for feature summary warning

---------

Co-authored-by: Daniel Smith <56164590+DanielRyanSmith@users.noreply.github.com>

.github/workflows/playwright.yml

@@ -17,7 +17,7 @@ jobs:
           with:
             node-version: 18
         - name: Run your tests
-          timeout-minutes: 15
+          timeout-minutes: 30
           run: npm run pwtests-shutdown --workspace=playwright; npm run test --workspace=playwright
         - uses: actions/upload-artifact@v3
           if: failure()

README.md

@@ -85,12 +85,19 @@ then you can update _all_ images for all tests with:
 npm run pwtests-update --workspace=playwright
 ```
 
-The updated images are also added to the __screenshots__ directory.
-If you change test file names, or test names, or screenshot image names, then
-you can manually delete the old screenshots, or simply delete all and update all.
+The updated images are also added to the __screenshots__ directory.  Images that
+did not need to be updated do not show up as having been changed.
+If you change the test file names, or the test method names, or the screenshot
+image file names, then new files will be generated, and you will need to manually delete the old files.  You could simply delete all screenshots and
+update all, but that will take a fairly long time.
 
-There is no way to run just one test or test file yet, but playwright supports
-doing that, so we should be able to add a parameter to the test running commands.
+You can update images for just one test file by adding `--filename=some_pwtest.js`
+to the `pwtests-update` command.  The `some_pwtest.js` name does not need to be
+a full path.
+
+If there are error reported by the GitHub CI playwright action, you can look at
+the error log, but if the problem is a difference in some of the images, you
+should probably download the artifact `.zip` file containing all the differences.
 
 There is some additional information for developers in developer-documentation.md.
 

client-src/elements/chromedash-app.js

@@ -236,7 +236,7 @@ class ChromedashApp extends LitElement {
       componentName);
     this.setUnsavedChanges(false);
     this.removeBeforeUnloadHandler();
-    this.pageComponent.allFormFieldComponents = {};
+    this.pageComponent.allFormFieldComponentsList = [];
 
     window.setTimeout(() => {
       // Timeout required since the form may not be created yet.

client-src/elements/chromedash-feature-detail.js

@@ -1,5 +1,5 @@
 import {LitElement, css, html, nothing} from 'lit';
-import {getStageValue, renderHTMLIf} from './utils';
+import {getFieldValueFromFeature, hasFieldValue, isDefinedValue, renderHTMLIf} from './utils';
 import {enhanceUrl} from './feature-link';
 import {openAddStageDialog} from './chromedash-add-stage-dialog';
 import {
@@ -21,12 +21,8 @@ import {
 import {
   DEPRECATED_FIELDS,
   GATE_TEAM_ORDER,
-  PLATFORMS_DISPLAYNAME,
-  STAGE_SPECIFIC_FIELDS,
-  OT_MILESTONE_END_FIELDS,
   STAGE_PSA_SHIPPING,
-  ENTERPRISE_FEATURE_CATEGORIES_DISPLAYNAME,
-  ROLLOUT_IMPACT_DISPLAYNAME} from './form-field-enums';
+} from './form-field-enums';
 import '@polymer/iron-icon';
 import './chromedash-activity-log';
 import './chromedash-callout';
@@ -289,110 +285,6 @@ class ChromedashFeatureDetail extends LitElement {
     `;
   }
 
-  isDefinedValue(value) {
-    return !(value === undefined || value === null || value.length == 0);
-  }
-
-  // Look at all extension milestones and calculate the highest milestone that an origin trial
-  // is available. This is used to display the highest milestone available, but to preserve the
-  // milestone that the trial was originally available for without extensions.
-  calcMaxMilestone(feStage, fieldName) {
-    // If the max milestone has already been calculated, or no trial extensions exist, do nothing.
-    if (feStage[`max_${fieldName}`] || !feStage.extensions) {
-      return;
-    }
-    let maxMilestone = getStageValue(feStage, fieldName) || 0;
-    for (const extension of feStage.extensions) {
-      const extensionValue = getStageValue(extension, fieldName);
-      if (extensionValue) {
-        maxMilestone = Math.max(maxMilestone, extensionValue);
-      }
-    }
-    // Save the findings with the "max_" prefix as a prop of the stage for reference.
-    feStage[`max_${fieldName}`] = maxMilestone;
-  }
-
-  // Get the milestone value that is displayed to the user regarding the origin trial end date.
-  getMilestoneExtensionValue(feStage, fieldName) {
-    const milestoneValue = getStageValue(feStage, fieldName);
-    this.calcMaxMilestone(feStage, fieldName);
-
-    const maxMilestoneFieldName = `max_${fieldName}`;
-    // Display only extension milestone if the original milestone has not been added.
-    if (feStage[maxMilestoneFieldName] && !milestoneValue) {
-      return `Extended to ${feStage[maxMilestoneFieldName]}`;
-    }
-    // If the trial has been extended past the original milestone, display the extension
-    // milestone with additional text reminding of the original milestone end date.
-    if (feStage[maxMilestoneFieldName] && feStage[maxMilestoneFieldName] > milestoneValue) {
-      return `${feStage[maxMilestoneFieldName]} (extended from ${milestoneValue})`;
-    }
-    return milestoneValue;
-  }
-
-  getFieldValue(fieldName, feStage) {
-    if (STAGE_SPECIFIC_FIELDS.has(fieldName)) {
-      const value = getStageValue(feStage, fieldName);
-      if (fieldName === 'rollout_impact' && value) {
-        return ROLLOUT_IMPACT_DISPLAYNAME[value];
-      } if (fieldName === 'rollout_platforms' && value) {
-        return value.map(platformId => PLATFORMS_DISPLAYNAME[platformId]);
-      } else if (fieldName in OT_MILESTONE_END_FIELDS) {
-        // If an origin trial end date is being displayed, handle extension milestones as well.
-        return this.getMilestoneExtensionValue(feStage, fieldName);
-      }
-      return value;
-    }
-
-    let value = this.feature[fieldName];
-    const fieldNameMapping = {
-      owner: 'browsers.chrome.owners',
-      editors: 'editors',
-      search_tags: 'tags',
-      spec_link: 'standards.spec',
-      standard_maturity: 'standards.maturity.text',
-      sample_links: 'resources.samples',
-      docs_links: 'resources.docs',
-      bug_url: 'browsers.chrome.bug',
-      blink_components: 'browsers.chrome.blink_components',
-      devrel: 'browsers.chrome.devrel',
-      prefixed: 'browsers.chrome.prefixed',
-      impl_status_chrome: 'browsers.chrome.status.text',
-      shipped_milestone: 'browsers.chrome.desktop',
-      shipped_android_milestone: 'browsers.chrome.android',
-      shipped_webview_milestone: 'browsers.chrome.webview',
-      shipped_ios_milestone: 'browsers.chrome.ios',
-      ff_views: 'browsers.ff.view.text',
-      ff_views_link: 'browsers.ff.view.url',
-      ff_views_notes: 'browsers.ff.view.notes',
-      safari_views: 'browsers.safari.view.text',
-      safari_views_link: 'browsers.safari.view.url',
-      safari_views_notes: 'browsers.safari.view.notes',
-      web_dev_views: 'browsers.webdev.view.text',
-      web_dev_views_link: 'browsers.webdev.view.url',
-      web_dev_views_notes: 'browsers.webdev.view.notes',
-      other_views_notes: 'browsers.other.view.notes',
-    };
-    if (fieldNameMapping[fieldName]) {
-      value = this.feature;
-      for (const step of fieldNameMapping[fieldName].split('.')) {
-        if (value) {
-          value = value[step];
-        }
-      }
-    }
-    if (fieldName === 'enterprise_feature_categories' && value) {
-      return value.map(categoryId =>
-        ENTERPRISE_FEATURE_CATEGORIES_DISPLAYNAME[categoryId]);
-    }
-    return value;
-  }
-
-  hasFieldValue(fieldName, feStage) {
-    const value = this.getFieldValue(fieldName, feStage);
-    return this.isDefinedValue(value);
-  }
-
   renderText(value) {
     value = String(value);
     const markup = autolink(value, this.featureLinks);
@@ -430,21 +322,21 @@ class ChromedashFeatureDetail extends LitElement {
 
   renderField(fieldDef, feStage) {
     const [fieldId, fieldDisplayName, fieldType] = fieldDef;
-    const value = this.getFieldValue(fieldId, feStage);
-    const isDefinedValue = this.isDefinedValue(value);
+    const value = getFieldValueFromFeature(fieldId, feStage, this.feature);
+    const isDefined = isDefinedValue(value);
     const isDeprecatedField = DEPRECATED_FIELDS.has(fieldId);
-    if (!isDefinedValue && isDeprecatedField) {
+    if (!isDefined && isDeprecatedField) {
       return nothing;
     }
 
-    const icon = isDefinedValue ?
+    const icon = isDefined ?
       html`<sl-icon library="material" name="check_circle_20px"></sl-icon>` :
       html`<sl-icon library="material" name="blank_20px"></sl-icon>`;
 
     return html`
       <dt id=${fieldId}>${icon} ${fieldDisplayName}</dt>
       <dd>
-       ${isDefinedValue ?
+       ${isDefined ?
           this.renderValue(fieldType, value) :
           html`<i>No information provided yet</i>`}
       </dd>
@@ -452,7 +344,7 @@ class ChromedashFeatureDetail extends LitElement {
   }
 
   stageHasAnyFilledFields(fields, feStage) {
-    return fields.some(fieldDef => this.hasFieldValue(fieldDef[0], feStage));
+    return fields.some(fieldDef => hasFieldValue(fieldDef[0], feStage, this.feature));
   }
 
   // Renders all fields for trial extension stages as a subsection of the

client-src/elements/chromedash-form-field.js

@@ -2,7 +2,7 @@ import {LitElement, html, nothing} from 'lit';
 import {ALL_FIELDS} from './form-field-specs';
 import {ref} from 'lit/directives/ref.js';
 import './chromedash-textarea';
-import {showToastMessage, getFieldValue} from './utils.js';
+import {showToastMessage, getFieldValueFromFeature} from './utils.js';
 
 export class ChromedashFormField extends LitElement {
   static get properties() {
@@ -11,7 +11,7 @@ export class ChromedashFormField extends LitElement {
       index: {type: Number}, // Represents which field this is on the form.
       stageId: {type: Number},
       value: {type: String},
-      fieldValues: {type: Array}, // All other field value objects
+      fieldValues: {type: Array}, // All other field value objects in current form.
       disabled: {type: Boolean},
       checkboxLabel: {type: String}, // Optional override of default label.
       shouldFadeIn: {type: Boolean},
@@ -36,6 +36,7 @@ export class ChromedashFormField extends LitElement {
     this.shouldFadeIn = false;
     this.loading = false;
     this.forEnterprise = false;
+    this.stageId = undefined;
     this.stageType = undefined;
     this.componentChoices = {};
     this.checkMessage = '';
@@ -44,6 +45,7 @@ export class ChromedashFormField extends LitElement {
   getValue() {
     // value can be a js or python boolean value converted to a string
     // or the initial value specified in form-field-spec
+    // If value is falsy, it will be replaced by the initial value, if any.
     return !this.value && this.fieldProps.initial ?
       this.fieldProps.initial : this.value;
   }
@@ -52,9 +54,10 @@ export class ChromedashFormField extends LitElement {
     super.connectedCallback();
     this.fieldProps = ALL_FIELDS[this.name] || {};
 
+    // Register this form field component with the page component.
     const app = document.querySelector('chromedash-app');
     if (app?.pageComponent) {
-      app.pageComponent.allFormFieldComponents[this.name] = this;
+      app.pageComponent.allFormFieldComponentsList.push(this);
     }
 
     if (this.name === 'blink_components') {
@@ -71,7 +74,20 @@ export class ChromedashFormField extends LitElement {
 
   firstUpdated() {
     this.initialValue = JSON.parse(JSON.stringify(this.value));
-    this.doSemanticCheck();
+
+    // We only want to do the following one time.
+    this.setupSemanticCheck();
+
+    // We need to wait until the entire page is rendered, so later dependents
+    // are available to do the semantic check, hence firstUpdated is too soon.
+    // Do first semantic check after the document is ready.
+    if (document.readyState === 'loading') {
+      document.addEventListener('DOMContentLoaded', () => (setTimeout(() => {
+        this.doSemanticCheck();
+      })));
+    } else {
+      this.doSemanticCheck();
+    }
   }
 
   updateAttributes(el) {
@@ -117,52 +133,101 @@ export class ChromedashFormField extends LitElement {
     };
     this.dispatchEvent(new CustomEvent('form-field-update', eventOptions));
 
-    // Run semantic check on this field.  Must be after above dispatch.
-    this.doSemanticCheck();
-    // Also doSemanticCheck on known dependent form fields.
+    // Run semantic checks on entire page.  Must be after above dispatch.
     const app = document.querySelector('chromedash-app');
-    const dependents = ALL_FIELDS[this.name].dependents;
-    if (dependents && app?.pageComponent) {
-      dependents.forEach((dependent) => {
-        const dependentField = app.pageComponent.allFormFieldComponents[dependent];
-        if (dependentField) {
-          dependentField.doSemanticCheck();
-        }
-      });
+    if (app?.pageComponent) {
+      app.pageComponent.allFormFieldComponentsList.forEach((formFieldComponent) =>
+        formFieldComponent.doSemanticCheck());
+    } else {
+      // Do the semantic check for unit testing.  Only works for isolated field.
+      this.doSemanticCheck();
     }
   }
 
-  async doSemanticCheck() {
-    const checkFunction = this.fieldProps.check;
-    if (checkFunction) {
-      const fieldValue = this.getValue();
-      const initialValue = this.initialValue;
-      const checkResult = await checkFunction(fieldValue,
-        (name) => getFieldValue(name, this.fieldValues), initialValue);
-      if (checkResult == null) {
-        this.checkMessage = '';
+  setupSemanticCheck() {
+    // Find the form-field component in order to set custom validity.
+    const fieldSelector = `#id_${this.name}`;
+    const formFieldElements = this.renderRoot.querySelectorAll(fieldSelector);
+    if (formFieldElements.length > 1) {
+      if (this.stageId) {
+        // The id is not unique for fields in multiple stages, e.g. origin trials.
+        // So let's try qualifying the selector with this.stageId in a container.
+        fieldSelector = `[stageId="${this.stageId} #id_${this.name}"]`;
+        formFieldElements = this.renderRoot.querySelectorAll(fieldSelector);
       } else {
-        this.checkMessage = html`
-          <span class="check-${
-              checkResult.message ? 'message' :
-              checkResult.warning ? 'warning' :
-                checkResult.error ? 'error' : 'unknown'
-            }">
-            ${
-              checkResult.message ? checkResult.message :
-              checkResult.warning ? html`<b>Warning</b>: ${checkResult.warning}` :
-                checkResult.error ? html`<b>Error</b>: ${checkResult.error}` :
-                  ''
-            }
-          </span>`;
-      }
-      const formFieldElement = this.renderRoot.querySelector(`#id_${this.name}`);
-      if (formFieldElement?.setCustomValidity &&
-        formFieldElement.input) {
-        formFieldElement.setCustomValidity(
-          (checkResult && checkResult.error) ? checkResult.error : '');
+        throw new Error(`Name of field, "${this.name}", is not unique and no stage Id was provided.`);
       }
     }
+    // There should only be one now.
+    const formFieldElement = formFieldElements[0];
+
+    // For 'input' elements.
+    if (formFieldElement?.setCustomValidity && formFieldElement.input) {
+      formFieldElement.setCustomValidity(
+        (checkResult && checkResult.error) ? checkResult.error : '');
+    }
+  // TODO: handle other form field types.
+  }
+
+  async doSemanticCheck() {
+    // Define function to get any other field value relative to this field.
+    // stageOrId is either a stage object or an id, or the special value
+    // 'current stage' which means use the same stage as for this field.
+    const getFieldValue = (fieldName, stageOrId) => {
+      if (stageOrId === 'current stage') {
+        stageOrId = this.stageId;
+      }
+      return getFieldValueWithStage(fieldName, stageOrId, this.fieldValues || []);
+    };
+    // Attach the feature to the getFieldValue function, which is needed to
+    // iterate through stages not in the form.
+    getFieldValue.feature = this.fieldValues?.feature;
+
+    const checkFunctionWrapper = async (checkFunction) => {
+      const fieldValue = this.getValue();
+      const initialValue = this.initialValue;
+
+      if (fieldValue == null) return false; // Assume there is nothing to check.
+
+      // Call the checkFunction and await result, in case it is async.
+      const checkResult = await checkFunction(fieldValue, getFieldValue, initialValue);
+      if (checkResult == null) {
+        // Don't clear this.checkMessage here.
+        return false;
+      } else {
+        this.checkMessage = html`
+          <span class="check-${checkResult.message ? 'message' :
+            checkResult.warning ? 'warning' :
+              checkResult.error ? 'error' : 'unknown'
+          }">
+            ${checkResult.message ? checkResult.message :
+            checkResult.warning ? html`<b>Warning</b>: ${checkResult.warning}` :
+              checkResult.error ? html`<b>Error</b>: ${checkResult.error}` :
+                ''
+          }
+          </span>`;
+        // Return from doSemanticCheck with the first non-empty message.
+        return true;
+      }
+    };
+
+    // Get the check function(s) to run.
+    const checkFunctionOrArray = this.fieldProps.check || [];
+    const checkFunctions =
+      (typeof checkFunctionOrArray === 'function') ?
+        [checkFunctionOrArray] : checkFunctionOrArray;
+    // If there are any check functions,
+    // then first clear this.checkMessage before running the checks.
+    if (checkFunctions.length > 0) {
+      this.checkMessage = '';
+    }
+
+    // Run each check function, and return after the first non-empty message.
+    for (const checkFunction of checkFunctions) {
+      if (await checkFunctionWrapper(checkFunction)) {
+        return;
+      }
+    };
   }
 
   renderWidget() {
@@ -234,6 +299,7 @@ export class ChromedashFormField extends LitElement {
           ${ref(this.updateAttributes)}
           name="${fieldName}"
           id="id_${this.name}"
+          stageId="${this.stageId}"
           size="small"
           autocomplete="off"
           .value=${fieldValue}
@@ -344,4 +410,49 @@ export class ChromedashFormField extends LitElement {
   }
 }
 
+
+/**
+ * Gets the value of a field from a feature entry form, or from the feature.
+ * Looks up the field name in the provided form field values, using the stageOrId
+ * if the field is stage-specific, and returns the corresponding value.
+ * Returns null if not defined or not found.
+ * Handles special cases like shipping milestones and mapped stage fields.
+ * @param {string} fieldName
+ * @param {number|Object|undefined} stageOrId
+ * @param {Array<{name:string, value:*}>} formFieldValues
+ * @return {*}
+ */
+function getFieldValueWithStage(fieldName, stageOrId, formFieldValues) {
+  // Iterate through formFieldValues looking for element with name==fieldName
+  // and stage == stageId, if there is a non-null stageId
+  let stageId;
+  if (typeof stageOrId === 'number') {
+    stageId = stageOrId;
+  } else if (typeof stageOrId === 'object') {
+    stageId = stageOrId.id;
+  }
+
+  for (const obj of formFieldValues) {
+    if (obj.name === fieldName && (stageId == null || obj.stageId == stageId)) {
+      return obj.value;
+    }
+  }
+
+  // The remainder looks for the field in the feature.
+  const feature = formFieldValues.feature;
+  if (feature == null) {
+    return null;
+  }
+
+  // Get the stage object for the field.
+  const feStage = (typeof stageOrId === 'object') ? stageOrId :
+    (stageId != null ?
+      feature.stages.find((s) => s.id == stageId) :
+      feature.stages[0]);
+
+  // Lookup fieldName by following the stage specific path starting from feature.
+  const value = getFieldValueFromFeature(fieldName, feStage, feature);
+  return value;
+}
+
 customElements.define('chromedash-form-field', ChromedashFormField);

client-src/elements/chromedash-form-field_test.js

@@ -74,27 +74,6 @@ describe('chromedash-form-field', () => {
     assert.include(renderElement.innerHTML, 'required');
   });
 
-  it('renders a warning about a field', async () => {
-    const component = await fixture(
-      html`
-      <chromedash-form-field name="summary" value="Very short summary">
-      </chromedash-form-field>`);
-    assert.exists(component);
-
-    const renderElement = component.renderRoot;
-    assert.include(renderElement.innerHTML, 'Feature summary should be');
-  });
-
-  it('renders without a warning for a valid field', async () => {
-    const component = await fixture(
-      html`
-      <chromedash-form-field name="summary" value="01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789">
-      </chromedash-form-field>`); // 110 chars
-    assert.exists(component);
-    const renderElement = component.renderRoot;
-    assert.notInclude(renderElement.innerHTML, 'Feature summary should be');
-  });
-
   it('renders a radios type of field', async () => {
     const component = await fixture(
       html`

client-src/elements/chromedash-guide-editall-page.js

@@ -255,6 +255,7 @@ export class ChromedashGuideEditallPage extends LitElement {
         <chromedash-form-field
           name=${field}
           index=${index}
+          stageId=${stageId}
           value=${value}
           .fieldValues=${this.fieldValues}
           ?forEnterprise=${formattedFeature.is_enterprise_feature}
@@ -378,7 +379,7 @@ export class ChromedashGuideEditallPage extends LitElement {
 
   renderForm() {
     const formattedFeature = formatFeatureForEdit(this.feature);
-    this.fieldValues.allFields = formattedFeature;
+    this.fieldValues.feature = this.feature;
 
     const formsToRender = this.getForms(formattedFeature, this.feature.stages);
     return html`

client-src/elements/chromedash-guide-metadata-page.js

@@ -191,7 +191,7 @@ export class ChromedashGuideMetadataPage extends LitElement {
 
   renderForm() {
     const formattedFeature = formatFeatureForEdit(this.feature);
-    this.fieldValues.allFields = formattedFeature;
+    this.fieldValues.feature = this.feature;
 
     let sections = FLAT_METADATA_FIELDS.sections;
     if (formattedFeature.is_enterprise_feature) {

client-src/elements/chromedash-guide-metadata.js

@@ -362,7 +362,7 @@ export class ChromedashGuideMetadata extends LitElement {
 
   renderEditForm() {
     const formattedFeature = formatFeatureForEdit(this.feature);
-    this.fieldValues.allFields = formattedFeature;
+    this.fieldValues.feature = this.feature;
 
     const metadataFields = flattenSections(this.feature.is_enterprise_feature ?
       FLAT_ENTERPRISE_METADATA_FIELDS :

client-src/elements/chromedash-guide-new-page.js

@@ -132,7 +132,7 @@ export class ChromedashGuideNewPage extends LitElement {
 
   renderForm() {
     const newFeatureInitialValues = {owner: this.userEmail};
-    this.fieldValues.allFields = newFeatureInitialValues;
+    this.fieldValues.feature = this.feature;
 
     const formFields = this.isEnterpriseFeature ?
       ENTERPRISE_NEW_FEATURE_FORM_FIELDS :

client-src/elements/chromedash-guide-stage-page.js

@@ -197,7 +197,7 @@ export class ChromedashGuideStagePage extends LitElement {
     `;
   }
 
-  renderFields(formattedFeature, section, feStage, useStageId=false) {
+  renderFields(formattedFeature, section, feStage) {
     if (!feStage) {
       feStage = this.stage;
     }
@@ -226,7 +226,7 @@ export class ChromedashGuideStagePage extends LitElement {
         index=${index}
         value=${value}
         .fieldValues=${this.fieldValues}
-        stageId=${useStageId ? feStage.id : undefined}
+        stageId=${feStage.id}
         ?forEnterprise=${formattedFeature.is_enterprise_feature}
         @form-field-update="${this.handleFormFieldUpdate}">
       </chromedash-form-field>
@@ -281,7 +281,7 @@ export class ChromedashGuideStagePage extends LitElement {
           formSections.push(html`
             <h3>${section.name} ${i}</h3>
             <section class="stage_form">
-              ${this.renderFields(formattedFeature, section, extensionStage, true)}
+              ${this.renderFields(formattedFeature, section, extensionStage)}
             </section>
           `);
         }
@@ -345,7 +345,7 @@ export class ChromedashGuideStagePage extends LitElement {
 
   renderForm() {
     const formattedFeature = formatFeatureForEdit(this.feature);
-    this.fieldValues.allFields = formattedFeature;
+    this.fieldValues.feature = this.feature;
 
     return html`
       <form name="feature_form">

client-src/elements/chromedash-guide-verify-accuracy-page.js

@@ -282,7 +282,7 @@ export class ChromedashGuideVerifyAccuracyPage extends LitElement {
 
   renderForm() {
     const formattedFeature = formatFeatureForEdit(this.feature);
-    this.fieldValues.allFields = formattedFeature;
+    this.fieldValues.feature = this.feature;
 
     const stageIds = this.getAllStageIds();
     const [allFormFields, formsToRender] = this.getForms(formattedFeature, this.feature.stages);

client-src/elements/chromedash-ot-creation-page.js

@@ -7,9 +7,7 @@ import {
   setupScrollToHash} from './utils.js';
 import './chromedash-form-table.js';
 import './chromedash-form-field.js';
-import {
-  formatFeatureForEdit,
-  ORIGIN_TRIAL_CREATION_FIELDS} from './form-definition.js';
+import {ORIGIN_TRIAL_CREATION_FIELDS} from './form-definition.js';
 import {SHARED_STYLES} from '../css/shared-css.js';
 import {FORM_STYLES} from '../css/forms-css.js';
 import {ALL_FIELDS} from './form-field-specs.js';
@@ -256,8 +254,8 @@ export class ChromedashOTCreationPage extends LitElement {
       return html`
       <chromedash-form-field
         name=${fieldInfo.name}
-        value=${fieldInfo.value}
         index=${i}
+        value=${fieldInfo.value}
         .fieldValues=${this.fieldValues}
         .shouldFadeIn=${shouldFadeIn}
         @form-field-update="${this.handleFormFieldUpdate}">
@@ -268,8 +266,7 @@ export class ChromedashOTCreationPage extends LitElement {
   }
 
   renderForm() {
-    const formattedFeature = formatFeatureForEdit(this.feature);
-    this.fieldValues.allFields = formattedFeature;
+    this.fieldValues.feature = this.feature;
 
     // OT creation page only has one section.
     const section = ORIGIN_TRIAL_CREATION_FIELDS.sections[0];

client-src/elements/chromedash-ot-extension-page.js

@@ -7,7 +7,6 @@ import {
 import './chromedash-form-table.js';
 import './chromedash-form-field.js';
 import {
-  formatFeatureForEdit,
   ORIGIN_TRIAL_EXTENSION_FIELDS} from './form-definition.js';
 import {OT_EXTENSION_STAGE_MAPPING} from './form-field-enums.js';
 import {ALL_FIELDS} from './form-field-specs';
@@ -225,8 +224,7 @@ export class ChromedashOTExtensionPage extends LitElement {
   }
 
   renderForm() {
-    const formattedFeature = formatFeatureForEdit(this.feature);
-    this.fieldValues.allFields = formattedFeature;
+    this.fieldValues.feature = this.feature;
 
     // OT extension page only has one section.
     const section = ORIGIN_TRIAL_EXTENSION_FIELDS.sections[0];

client-src/elements/form-field-enums.js

@@ -212,6 +212,27 @@ export const INTENT_STAGES = {
   INTENT_ROLLOUT: [10, 'Rollout'],
 };
 
+export const DT_MILESTONE_FIELDS = new Set([
+  'dt_milestone_desktop_start',
+  'dt_milestone_android_start',
+  'dt_milestone_ios_start',
+  'dt_milestone_webview_start',
+]);
+
+export const OT_MILESTONE_START_FIELDS = new Set([
+  'ot_milestone_desktop_start',
+  'ot_milestone_android_start',
+  'ot_milestone_webview_start',
+  // 'ot_milestone_ios_start',
+]);
+
+export const SHIPPED_MILESTONE_FIELDS = new Set([
+  'shipped_milestone',
+  'shipped_android_milestone',
+  'shipped_ios_milestone',
+  'shipped_webview_milestone',
+]);
+
 // Every mutable field that exists on the Stage entity and every key
 // in MilestoneSet.MILESTONE_FIELD_MAPPING should be listed here.
 export const STAGE_SPECIFIC_FIELDS = new Set([

client-src/elements/form-field-specs.js

@@ -12,6 +12,12 @@ import {
   VENDOR_VIEWS_COMMON,
   VENDOR_VIEWS_GECKO,
   WEB_DEV_VIEWS,
+  DT_MILESTONE_FIELDS,
+  OT_MILESTONE_START_FIELDS,
+  SHIPPED_MILESTONE_FIELDS,
+  STAGE_TYPES_DEV_TRIAL,
+  STAGE_TYPES_ORIGIN_TRIAL,
+  STAGE_TYPES_SHIPPING,
 } from './form-field-enums';
 
 /* Patterns from https://www.oreilly.com/library/view/regular-expressions-cookbook/9781449327453/ch04s01.html
@@ -77,55 +83,78 @@ const OT_MILESTONE_WEBVIEW_RANGE = {
   later: 'ot_milestone_webview_end',
 };
 
-export const OT_SHIPPED_MILESTONE_DESKTOP_RANGE = {
+const OT_ALL_SHIPPED_MILESTONE_DESKTOP_RANGE = {
   earlier: 'ot_milestone_desktop_start',
-  later: 'shipped_milestone',
-  error: 'Origin trial must start before feature is shipped.',
+  allLater: 'shipped_milestone',
+  warning: 'Origin trial starting milestone should be before all feature shipping milestones.',
 };
 
-const OT_SHIPPED_MILESTONE_WEBVIEW_RANGE = {
+const ALL_OT_SHIPPED_MILESTONE_DESKTOP_RANGE = {
+  allEarlier: 'ot_milestone_desktop_start',
+  later: 'shipped_milestone',
+  warning: 'All origin trials starting milestones should be before feature shipping milestone.',
+};
+
+const OT_ALL_SHIPPED_MILESTONE_WEBVIEW_RANGE = {
   earlier: 'ot_milestone_webview_start',
-  later: 'shipped_webview_milestone',
-  error: 'Origin trial must start before feature is shipped.',
+  allLater: 'shipped_webview_milestone',
+  warning: 'Origin trial starting milestone should be before all feature shipping milestones.',
 };
 
-const OT_SHIPPED_MILESTONE_ANDROID_RANGE = {
+const ALL_OT_SHIPPED_MILESTONE_WEBVIEW_RANGE = {
+  allEarlier: 'ot_milestone_webview_start',
+  later: 'shipped_webview_milestone',
+  warning: 'All origin trials starting milestones should be before feature shipping milestone.',
+};
+
+const OT_ALL_SHIPPED_MILESTONE_ANDROID_RANGE = {
   earlier: 'ot_milestone_android_start',
+  allLater: 'shipped_android_milestone',
+  warning: 'Origin trial starting milestone should be before all feature shipping milestones.',
+};
+
+const ALL_OT_SHIPPED_MILESTONE_ANDROID_RANGE = {
+  allEarlier: 'ot_milestone_android_start',
   later: 'shipped_android_milestone',
-  error: 'Origin trial must start before feature is shipped.',
+  warning: 'All origin trials starting milestones should be before feature shipping milestone.',
 };
 
-const OT_SHIPPED_MILESTONE_IOS_RANGE = {
-  earlier: 'ot_milestone_ios_start',
-  later: 'shipped_ios_milestone',
-  error: 'Origin trial must start before feature is shipped.',
-};
-
-const DT_SHIPPED_MILESTONE_DESKTOP_RANGE = {
+const DT_ALL_SHIPPED_MILESTONE_DESKTOP_RANGE = {
   earlier: 'dt_milestone_desktop_start',
+  allLater: 'shipped_milestone',
+  warning: 'Shipped milestone should be later than dev trial.',
+};
+
+const ALL_DT_SHIPPED_MILESTONE_DESKTOP_RANGE = {
+  allEarlier: 'dt_milestone_desktop_start',
   later: 'shipped_milestone',
-  error: 'Shipped milestone must be later than dev trial.',
+  warning: 'Shipped milestone should be later than dev trial.',
 };
 
-const DT_SHIPPED_MILESTONE_ANDROID_RANGE = {
+const DT_ALL_SHIPPED_MILESTONE_ANDROID_RANGE = {
   earlier: 'dt_milestone_android_start',
+  allLater: 'shipped_android_milestone',
+  warning: 'Shipped milestone should be later than dev trial start milestone.',
+};
+
+const ALL_DT_SHIPPED_MILESTONE_ANDROID_RANGE = {
+  allEarlier: 'dt_milestone_android_start',
   later: 'shipped_android_milestone',
-  error: 'Shipped milestone must be later than dev trial.',
+  warning: 'Shipped milestone should be later than dev trial start milestone.',
 };
 
-const DT_SHIPPED_MILESTONE_IOS_RANGE = {
+const DT_ALL_SHIPPED_MILESTONE_IOS_RANGE = {
   earlier: 'dt_milestone_ios_start',
+  allLater: 'shipped_ios_milestone',
+  warning: 'Shipped milestone should be later than dev trial start milestone.',
+};
+
+const ALL_DT_SHIPPED_MILESTONE_IOS_RANGE = {
+  allEarlier: 'dt_milestone_ios_start',
   later: 'shipped_ios_milestone',
-  error: 'Shipped milestone must be later than dev trial.',
+  warning: 'Shipped milestone should be later than dev trial start milestone.',
 };
 
-const DT_SHIPPED_MILESTONE_WEBVIEW_RANGE = {
-  earlier: 'dt_milestone_webview_start',
-  later: 'shipped_webview_milestone',
-  error: 'Shipped webview milestone must be later than dev trial.',
-};
-
-
 const MULTI_URL_FIELD_ATTRS = {
   title: 'Enter one or more full URLs, one per line:\nhttps://...\nhttps://...',
   multiple: true,
@@ -174,7 +203,6 @@ export const ALL_FIELDS = {
     </ul>`,
     check: (_value, getFieldValue) =>
       checkFeatureNameAndType(getFieldValue),
-    dependents: ['feature_type', 'feature_type_radio_group'],
   },
 
   'summary': {
@@ -321,7 +349,6 @@ export const ALL_FIELDS = {
         cannot be changed. If this field needs to be modified, a new feature
         would need to be created.</p>`,
     check: (_value, getFieldValue) => checkFeatureNameAndType(getFieldValue),
-    dependents: ['name'],
   },
 
   'feature_type_radio_group': {
@@ -337,7 +364,6 @@ export const ALL_FIELDS = {
         cannot be changed. If this field needs to be modified, a new feature
         would need to be created.</p>`,
     check: (_value, getFieldValue) => checkFeatureNameAndType(getFieldValue),
-    dependents: ['name'],
   },
 
   'set_stage': {
@@ -1029,9 +1055,8 @@ export const ALL_FIELDS = {
     check: (_value, getFieldValue) =>
       checkMilestoneRanges([
         OT_MILESTONE_DESKTOP_RANGE,
-        OT_SHIPPED_MILESTONE_DESKTOP_RANGE,
+        OT_ALL_SHIPPED_MILESTONE_DESKTOP_RANGE,
       ], getFieldValue),
-    dependents: ['ot_milestone_desktop_end', 'shipped_milestone'],
   },
 
   'ot_milestone_desktop_end': {
@@ -1044,7 +1069,6 @@ export const ALL_FIELDS = {
       trial of this feature.`,
     check: (_value, getFieldValue) =>
       checkMilestoneRanges([OT_MILESTONE_DESKTOP_RANGE], getFieldValue),
-    dependents: ['ot_milestone_desktop_start'],
   },
 
   'ot_milestone_android_start': {
@@ -1058,9 +1082,7 @@ export const ALL_FIELDS = {
     check: (_value, getFieldValue) =>
       checkMilestoneRanges([
         OT_MILESTONE_ANDROID_RANGE,
-        OT_SHIPPED_MILESTONE_ANDROID_RANGE], getFieldValue),
-    dependents: ['ot_milestone_android_end', 'shipped_android_milestone'],
-
+        OT_ALL_SHIPPED_MILESTONE_ANDROID_RANGE], getFieldValue),
   },
 
   'ot_milestone_android_end': {
@@ -1073,7 +1095,6 @@ export const ALL_FIELDS = {
       trial of this feature.`,
     check: (_value, getFieldValue) =>
       checkMilestoneRanges([OT_MILESTONE_ANDROID_RANGE], getFieldValue),
-    dependents: ['ot_milestone_android_start'],
   },
 
   'ot_milestone_webview_start': {
@@ -1087,8 +1108,7 @@ export const ALL_FIELDS = {
     check: (_value, getFieldValue) =>
       checkMilestoneRanges([
         OT_MILESTONE_WEBVIEW_RANGE,
-        OT_SHIPPED_MILESTONE_IOS_RANGE], getFieldValue),
-    dependents: ['ot_milestone_webview_end', 'shipped_ios_milestone'],
+        OT_ALL_SHIPPED_MILESTONE_WEBVIEW_RANGE], getFieldValue),
   },
 
   'ot_milestone_webview_end': {
@@ -1101,7 +1121,6 @@ export const ALL_FIELDS = {
       trial of this feature.`,
     check: (_value, getFieldValue) =>
       checkMilestoneRanges([OT_MILESTONE_WEBVIEW_RANGE], getFieldValue),
-    dependents: ['ot_milestone_ios_start'],
   },
 
   'experiment_risks': {
@@ -1340,9 +1359,8 @@ export const ALL_FIELDS = {
     check: (_value, getFieldValue) =>
       checkMilestoneRanges([
         OT_MILESTONE_DESKTOP_RANGE,
-        OT_SHIPPED_MILESTONE_DESKTOP_RANGE,
+        OT_ALL_SHIPPED_MILESTONE_DESKTOP_RANGE,
       ], getFieldValue),
-    dependents: ['ot_milestone_desktop_end', 'shipped_milestone'],
   },
 
   'ot_creation__milestone_desktop_last': {
@@ -1356,7 +1374,6 @@ export const ALL_FIELDS = {
       trial of this feature.`,
     check: (_value, getFieldValue) =>
       checkMilestoneRanges([OT_MILESTONE_DESKTOP_RANGE], getFieldValue),
-    dependents: ['ot_milestone_desktop_start'],
   },
 
   'anticipated_spec_changes': {
@@ -1517,10 +1534,9 @@ export const ALL_FIELDS = {
     help_text: SHIPPED_HELP_TXT,
     check: (_value, getFieldValue) =>
       checkMilestoneRanges([
-        OT_SHIPPED_MILESTONE_DESKTOP_RANGE,
-        DT_SHIPPED_MILESTONE_DESKTOP_RANGE], getFieldValue),
-    dependents: [
-      'dt_milestone_desktop_start', 'ot_milestone_desktop_start', 'shipped_milestone'],
+        ALL_OT_SHIPPED_MILESTONE_DESKTOP_RANGE,
+        ALL_DT_SHIPPED_MILESTONE_DESKTOP_RANGE,
+      ], getFieldValue),
   },
 
   'shipped_android_milestone': {
@@ -1530,10 +1546,9 @@ export const ALL_FIELDS = {
     label: 'Chrome for Android',
     help_text: SHIPPED_HELP_TXT,
     check: (_value, getFieldValue) =>
-      checkMilestoneRanges([OT_SHIPPED_MILESTONE_ANDROID_RANGE,
-        DT_SHIPPED_MILESTONE_ANDROID_RANGE], getFieldValue),
-    dependents: [
-      'dt_milestone_android_start', 'ot_milestone_android_start', 'shipped_android_milestone'],
+      checkMilestoneRanges([
+        ALL_OT_SHIPPED_MILESTONE_ANDROID_RANGE,
+        ALL_DT_SHIPPED_MILESTONE_ANDROID_RANGE], getFieldValue),
   },
 
   'shipped_ios_milestone': {
@@ -1544,10 +1559,7 @@ export const ALL_FIELDS = {
     help_text: SHIPPED_HELP_TXT,
     check: (_value, getFieldValue) =>
       checkMilestoneRanges([
-        OT_SHIPPED_MILESTONE_IOS_RANGE,
-        DT_SHIPPED_MILESTONE_IOS_RANGE], getFieldValue),
-    dependents: [
-      'dt_milestone_ios_start', 'ot_milestone_ios_start', 'shipped_ios_milestone'],
+        ALL_DT_SHIPPED_MILESTONE_IOS_RANGE], getFieldValue),
   },
 
   'shipped_webview_milestone': {
@@ -1558,10 +1570,7 @@ export const ALL_FIELDS = {
     help_text: SHIPPED_WEBVIEW_HELP_TXT,
     check: (_value, getFieldValue) =>
       checkMilestoneRanges([
-        OT_SHIPPED_MILESTONE_WEBVIEW_RANGE,
-        DT_SHIPPED_MILESTONE_WEBVIEW_RANGE], getFieldValue),
-    dependents: [
-      'dt_milestone_webview_start', 'ot_milestone_webview_start', 'shipped_webview_milestone'],
+        ALL_OT_SHIPPED_MILESTONE_WEBVIEW_RANGE], getFieldValue),
   },
 
   'requires_embedder_support': {
@@ -1607,8 +1616,8 @@ export const ALL_FIELDS = {
       When flags are enabled by default in preparation for
       shipping or removal, please use the fields in the ship stage.`,
     check: (_value, getFieldValue) =>
-      checkMilestoneRanges([DT_SHIPPED_MILESTONE_DESKTOP_RANGE], getFieldValue),
-    dependents: ['dt_milestone_desktop_start', 'shipped_milestone'],
+      checkMilestoneRanges([
+        DT_ALL_SHIPPED_MILESTONE_DESKTOP_RANGE], getFieldValue),
   },
 
   'dt_milestone_android_start': {
@@ -1622,8 +1631,7 @@ export const ALL_FIELDS = {
       When flags are enabled by default in preparation for
       shipping or removal, please use the fields in the ship stage.`,
     check: (_value, getFieldValue) =>
-      checkMilestoneRanges([DT_SHIPPED_MILESTONE_ANDROID_RANGE], getFieldValue),
-    dependents: ['dt_milestone_android_start', 'shipped_android_milestone'],
+      checkMilestoneRanges([DT_ALL_SHIPPED_MILESTONE_ANDROID_RANGE], getFieldValue),
   },
 
   'dt_milestone_ios_start': {
@@ -1637,8 +1645,7 @@ export const ALL_FIELDS = {
       When flags are enabled by default in preparation for
       shipping or removal, please use the fields in the ship stage.`,
     check: (_value, getFieldValue) =>
-      checkMilestoneRanges([DT_SHIPPED_MILESTONE_IOS_RANGE], getFieldValue),
-    dependents: ['dt_milestone_ios_start', 'shipped_ios_milestone'],
+      checkMilestoneRanges([DT_ALL_SHIPPED_MILESTONE_IOS_RANGE], getFieldValue),
   },
 
   'flag_name': {
@@ -1856,24 +1863,116 @@ export function makeDisplaySpecs(fieldNames) {
   return fieldNames.map(fieldName => makeDisplaySpec(fieldName));
 }
 
-function checkMilestoneRanges(ranges, getFieldValue) {
-  const getValue = (name) => {
-    const value = getFieldValue(name);
-    if (typeof value === 'string') {
-      if (value === '') return undefined;
-      return Number(value);
-    }
-  };
-  for (const range of ranges) {
-    const {earlier, later, error} = range;
-    const earlierMilestone = getValue(earlier);
-    const laterMilestone = getValue(later);
-    if (earlierMilestone != null && laterMilestone != null) {
-      if (laterMilestone <= earlierMilestone) {
-        return {error: error || 'Start milestone must be before end milestone'};
+// Find the minimum milestone, used for shipped milestones.
+function findMinMilestone(fieldName, stageTypes, getFieldValue) {
+  let minMilestone = Infinity;
+  // Iterate through all stages that are in stageTypes.
+  const feature = getFieldValue.feature;
+  for (const stage of feature.stages) {
+    if (stageTypes.has(stage.stage_type)) {
+      const milestone = getFieldValue(fieldName, stage);
+      if (milestone != null && milestone !== '') {
+        minMilestone = Math.min(minMilestone, milestone);
       }
     }
   }
+  if (minMilestone === Infinity) return undefined;
+  return minMilestone;
+}
+
+// Find the maximum milestone, used for OT start milestones.
+function findMaxMilestone(fieldName, stageTypes, getFieldValue) {
+  let maxMilestone = -Infinity;
+  // Iterate through all stages that are in stageTypes.
+  const feature = getFieldValue.feature;
+  for (const stage of feature.stages) {
+    if (stageTypes.has(stage.stage_type)) {
+      const milestone = getFieldValue(fieldName, stage);
+      if (milestone != null && milestone !== '') {
+        maxMilestone = Math.max(maxMilestone, milestone);
+      }
+    }
+  }
+  if (maxMilestone === -Infinity) return undefined;
+  return maxMilestone;
+}
+
+// Check that the earlier milestone is before all later milestones.
+// Used with OT start milestone and all shipped milestones.
+function checkEarlierBeforeAllLaterMilestones(
+  fieldPair, getFieldValue) {
+  const {earlier, allLater, warning} = fieldPair;
+  const stageTypes =
+    // Only shipping, for now.
+    SHIPPED_MILESTONE_FIELDS.has(allLater) ? STAGE_TYPES_SHIPPING : null;
+  const earlierValue = getNumericValue(earlier, getFieldValue);
+  const laterValue = findMinMilestone(allLater, stageTypes, getFieldValue);
+  if (earlierValue != null && laterValue != null &&
+    (Number(earlierValue) >= laterValue)) {
+    return warning ? {
+      warning,
+    } : {
+      error: error || `Earlier milestone #${earlierValue} should be before shipped milestone #${laterValue}.`,
+    };
+  }
+}
+
+// Check that all earlier milestones before a later milestone.
+// Used with all OT start milestones and a shipped milestone.
+function checkAllEarlierBeforeLaterMilestone(fieldPair, getFieldValue) {
+  const {allEarlier, later, warning} = fieldPair;
+  const stageTypes =
+    // Only origin trials or dev trials, for now.
+    OT_MILESTONE_START_FIELDS.has(allEarlier) ? STAGE_TYPES_ORIGIN_TRIAL :
+      DT_MILESTONE_FIELDS.has(allEarlier) ? STAGE_TYPES_DEV_TRIAL : null;
+  // console.info(`stageTypes: ${stageTypes}`);
+  const earlierValue = findMaxMilestone(allEarlier, stageTypes, getFieldValue);
+  const laterValue = getNumericValue(later, getFieldValue);
+  // console.info(`Earlier: ${earlierValue} ... later: ${laterValue}`);
+  if (earlierValue != null && laterValue != null &&
+    (earlierValue >= Number(laterValue))) {
+    return warning ? {
+      warning,
+    } : {
+      error: error || `Earlier milestone #${earlierValue} should be before shipped milestone #${laterValue}.`,
+    };
+  }
+}
+
+function getNumericValue(name, getFieldValue) {
+  const value = getFieldValue(name, 'current stage');
+  if (typeof value === 'string') {
+    if (value === '') return undefined;
+    return Number(value);
+  }
+  return value;
+};
+
+function checkMilestoneRanges(ranges, getFieldValue) {
+  let result;
+  for (const range of ranges) {
+    const { earlier, allEarlier, later, allLater, warning, error } = range;
+    // There can be an allLater or allEarlier, but not both.
+    if (allLater) {
+      result = checkEarlierBeforeAllLaterMilestones(range, getFieldValue);
+    } else if (allEarlier) {
+      result = checkAllEarlierBeforeLaterMilestone(range, getFieldValue);
+    } else {
+      const earlierMilestone = getNumericValue(earlier, getFieldValue);
+      const laterMilestone = getNumericValue(later, getFieldValue);
+      if (earlierMilestone != null && laterMilestone != null) {
+        if (laterMilestone <= earlierMilestone) {
+          // It's either a warning or an error.
+          result = warning ? {
+            warning,
+          } : {
+            error: error || 'Start milestone must be before end milestone',
+          };
+        }
+      }
+    }
+    if (result) return result;
+  }
 }
 
 function checkFeatureNameAndType(getFieldValue) {

client-src/elements/utils.js

@@ -2,7 +2,14 @@
 
 import {markupAutolinks} from './autolink.js';
 import {nothing, html} from 'lit';
-import {STAGE_FIELD_NAME_MAPPING} from './form-field-enums';
+import {
+  STAGE_FIELD_NAME_MAPPING,
+  PLATFORMS_DISPLAYNAME,
+  STAGE_SPECIFIC_FIELDS,
+  OT_MILESTONE_END_FIELDS,
+  ENTERPRISE_FEATURE_CATEGORIES_DISPLAYNAME,
+  ROLLOUT_IMPACT_DISPLAYNAME,
+} from './form-field-enums';
 
 let toastEl;
 
@@ -93,12 +100,144 @@ export function findFirstFeatureStage(intentStage, currentStage, fe) {
 
 /* Get the value of a stage field using a form-specific name */
 export function getStageValue(stage, fieldName) {
+  if (!stage) return undefined;
   if (fieldName in STAGE_FIELD_NAME_MAPPING) {
     return stage[STAGE_FIELD_NAME_MAPPING[fieldName]];
   }
   return stage[fieldName];
 }
 
+// Look at all extension milestones and calculate the highest milestone that an origin trial
+// is available. This is used to display the highest milestone available, but to preserve the
+// milestone that the trial was originally available for without extensions.
+function calcMaxMilestone(feStage, fieldName) {
+  // If the max milestone has already been calculated, or no trial extensions exist, do nothing.
+  if (!feStage) return;
+  if (feStage[`max_${fieldName}`] || !feStage.extensions) {
+    return;
+  }
+  let maxMilestone = getStageValue(feStage, fieldName) || 0;
+  for (const extension of feStage.extensions) {
+    const extensionValue = getStageValue(extension, fieldName);
+    if (extensionValue) {
+      maxMilestone = Math.max(maxMilestone, extensionValue);
+    }
+  }
+  // Save the findings with the "max_" prefix as a prop of the stage for reference.
+  feStage[`max_${fieldName}`] = maxMilestone;
+}
+
+
+// Get the milestone value that is displayed to the user regarding the origin trial end date.
+function getMilestoneExtensionValue(feStage, fieldName) {
+  if (!feStage) return undefined;
+  const milestoneValue = getStageValue(feStage, fieldName);
+  calcMaxMilestone(feStage, fieldName);
+
+  const maxMilestoneFieldName = `max_${fieldName}`;
+  // Display only extension milestone if the original milestone has not been added.
+  if (feStage[maxMilestoneFieldName] && !milestoneValue) {
+    return `Extended to ${feStage[maxMilestoneFieldName]}`;
+  }
+  // If the trial has been extended past the original milestone, display the extension
+  // milestone with additional text reminding of the original milestone end date.
+  if (feStage[maxMilestoneFieldName] && feStage[maxMilestoneFieldName] > milestoneValue) {
+    return `${feStage[maxMilestoneFieldName]} (extended from ${milestoneValue})`;
+  }
+  return milestoneValue;
+}
+
+/**
+ * Check if a value is defined and not empty.
+ *
+ * @param {any} value - The value to be checked.
+ * @return {boolean} Returns true if the value is defined and not empty, otherwise false.
+ */
+export function isDefinedValue(value) {
+  return !(value === undefined || value === null || value.length == 0);
+}
+
+export function hasFieldValue(fieldName, feStage, feature) {
+  const value = getFieldValueFromFeature(fieldName, feStage, feature);
+  return isDefinedValue(value);
+}
+
+/**
+ * Retrieves the value of a specific field for a given feature.
+ * Note: This is independent of any value that might be in a corresponding
+ * form field.
+ *
+ * @param {string} fieldName - The name of the field to retrieve.
+ * @param {string} feStage - The stage of the feature.
+ * @param {Object} feature - The feature object to retrieve the field value from.
+ * @return {*} The value of the specified field for the given feature.
+ */
+export function getFieldValueFromFeature(fieldName, feStage, feature) {
+  if (STAGE_SPECIFIC_FIELDS.has(fieldName)) {
+    const value = getStageValue(feStage, fieldName);
+    if (fieldName === 'rollout_impact' && value) {
+      return ROLLOUT_IMPACT_DISPLAYNAME[value];
+    } if (fieldName === 'rollout_platforms' && value) {
+      return value.map(platformId => PLATFORMS_DISPLAYNAME[platformId]);
+    } else if (fieldName in OT_MILESTONE_END_FIELDS) {
+      // If an origin trial end date is being displayed, handle extension milestones as well.
+      return getMilestoneExtensionValue(feStage, fieldName);
+    }
+    return value;
+  }
+
+  if (!feature) {
+    return null;
+  }
+
+  const fieldNameMapping = {
+    owner: 'browsers.chrome.owners',
+    editors: 'editors',
+    search_tags: 'tags',
+    spec_link: 'standards.spec',
+    standard_maturity: 'standards.maturity.text',
+    sample_links: 'resources.samples',
+    docs_links: 'resources.docs',
+    bug_url: 'browsers.chrome.bug',
+    blink_components: 'browsers.chrome.blink_components',
+    devrel: 'browsers.chrome.devrel',
+    prefixed: 'browsers.chrome.prefixed',
+    impl_status_chrome: 'browsers.chrome.status.text',
+    shipped_milestone: 'browsers.chrome.desktop',
+    shipped_android_milestone: 'browsers.chrome.android',
+    shipped_webview_milestone: 'browsers.chrome.webview',
+    shipped_ios_milestone: 'browsers.chrome.ios',
+    ff_views: 'browsers.ff.view.text',
+    ff_views_link: 'browsers.ff.view.url',
+    ff_views_notes: 'browsers.ff.view.notes',
+    safari_views: 'browsers.safari.view.text',
+    safari_views_link: 'browsers.safari.view.url',
+    safari_views_notes: 'browsers.safari.view.notes',
+    web_dev_views: 'browsers.webdev.view.text',
+    web_dev_views_link: 'browsers.webdev.view.url',
+    web_dev_views_notes: 'browsers.webdev.view.notes',
+    other_views_notes: 'browsers.other.view.notes',
+  };
+  let value;
+  if (fieldNameMapping[fieldName]) {
+    let propertyValue = feature;
+    for (const step of fieldNameMapping[fieldName].split('.')) {
+      if (propertyValue) {
+        propertyValue = propertyValue[step];
+      }
+    }
+    value = propertyValue;
+  } else {
+    value = feature[fieldName];
+  }
+
+  if (fieldName === 'enterprise_feature_categories' && value) {
+    return value.map(categoryId =>
+      ENTERPRISE_FEATURE_CATEGORIES_DISPLAYNAME[categoryId]);
+  }
+  return value;
+}
+
 /* Given a stage form definition, return a flat array of the fields associated with the stage. */
 export function flattenSections(stage) {
   return stage.sections.reduce((combined, section) => [...combined, ...section.fields], []);
@@ -365,20 +504,3 @@ export function handleSaveChangesResponse(response) {
   const app = document.querySelector('chromedash-app');
   app.setUnsavedChanges(response !== '');
 }
-
-/**
- * Returns value for fieldName, retrieved from fieldValues.
-  * @param {string} fieldName
-  * @param {Array<Object>} formFieldValues, with allFields property for everything else
-  * @return {*} The value of the named field.
- */
-export function getFieldValue(fieldName, formFieldValues) {
-  let fieldValue = formFieldValues.allFields ? formFieldValues[fieldName] : null;
-  formFieldValues.some((fieldObj) => {
-    if (fieldObj.name === fieldName) {
-      fieldValue = fieldObj.value;
-      return true;
-    }
-  });
-  return fieldValue;
-}

packages/playwright/package.json

@@ -7,7 +7,7 @@
         "//pwtests": "The version of playwright used here in devDependencies:@playwright/test must be the same as the docker image version used in Dockerfile, so the '^' prefix should not be included to avoid auto-upgrade of playwright which would then be out of sync.",
         "test": "npm run pwtests",
         "pwtests": "npm run pwtests-shutdown; ./run.sh bash -c \"./wait-for-app.sh && npx playwright test\"",
-        "pwtests-update": "npm run pwtests-shutdown; ./run.sh bash -c \"./wait-for-app.sh && npx playwright test --update-snapshots\"",
+        "pwtests-update": "npm run pwtests-shutdown; ./run.sh bash -c \"./wait-for-app.sh && npx playwright test --update-snapshots $npm_config_filename \"",
         "pwtests-report": "npm run pwtests-shutdown; ./run.sh bash -c \"./wait-for-app.sh && npx playwright show-report\"",
         "pwtests-ui": "npm run pwtests-shutdown; ./run.sh bash -c \"./wait-for-app.sh && npx playwright test --ui --ui-port=8123\"",
         "pwtests-shell": "./run.sh bash",

packages/playwright/playwright.config.cjs

@@ -18,15 +18,15 @@ module.exports = defineConfig({
   snapshotPathTemplate: '{testDir}/{testFileDir}/__screenshots__/{testFileName}/{testName}/{arg}-{projectName}-{platform}{ext}',
 
   /* Run tests in files in parallel */
-  fullyParallel: true,
+  fullyParallel: false,
   /* Fail the build on CI if you accidentally left test.only in the source code. */
   forbidOnly: !!process.env.CI,
-  /* Retry on CI only */
-  retries: process.env.CI ? 2 : 0,
-  /* Opt out of parallel tests. */
-  workers: process.env.CI ? 1 : undefined,
-  /* Reporter to use. Not for CI. See https://playwright.dev/docs/test-reporters */
-  reporter: process.env.CI ? [ ['html', { open: 'never'}] ] : [ ['html', { open: 'always'}] ],
+  /* Retries should only be needed for flakey tests, which we should avoid. */
+  retries: 0, // process.env.CI ? 2 : 0,
+  /* Opt out of parallel tests, since we cannot avoid shared sessions ATM. */
+  workers: 1, // was: process.env.CI ? 1 : undefined,
+  /* Use pwtests-report instead of reporter: 'html'.  Not for CI. See https://playwright.dev/docs/test-reporters */
+  reporter: 'line', /* was: process.env.CI ? [ ['html', { open: 'never'}] ] : [ ['html', { open: 'always'}] ], */
   /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */
   use: {
     /* Base URL to use in actions like `await page.goto('/')`. */

packages/playwright/tests/chromedash-guide-edit-page_pwtest.js

@@ -0,0 +1,41 @@
+// @ts-check
+import { test, expect } from '@playwright/test';
+import { editFeature, captureConsoleMessages, login, logout, createNewFeature } from './test_utils';
+
+
+test.beforeEach(async ({ page }, testInfo) => {
+    captureConsoleMessages(page);
+    testInfo.setTimeout(90000);
+
+    // Login before running each test.
+    await login(page);
+});
+
+test.afterEach(async ({ page }) => {
+    // Logout after running each test.
+    await logout(page);
+});
+
+
+test('edit feature', async ({ page }) => {
+    await createNewFeature(page);
+    await editFeature(page);
+
+    // Screenshot editor page
+    await expect(page).toHaveScreenshot('new-feature-edit.png');
+
+    // The following causes flakey errors.
+    // // Register to accept the confirm dialog before clicking to delete.
+    // page.once('dialog', dialog => dialog.accept());
+
+    //   // Delete the new feature.
+    // const deleteButton = page.locator('a[id$="delete-feature"]');
+    // await deleteButton.click();
+    // await delay(500);
+
+    // Screenshot the feature list after deletion.
+    // Not yet, since deletion only marks the feature as deleted,
+    // and the resulting page is always different.
+    // await expect(page).toHaveScreenshot('new-feature-deleted.png');
+    // await delay(500);
+});

packages/playwright/tests/chromedash-guide-editall-page_pwtest.js

@@ -0,0 +1,75 @@
+// @ts-check
+import { test, expect } from '@playwright/test';
+import { captureConsoleMessages, delay, login, logout, createNewFeature, deleteFeature } from './test_utils';
+
+
+/**
+ * Starting from the feature page, goto 'Edit all fields'.
+ * @param {import('@playwright/test').Page} page
+ */
+async function gotoEditAllPage(page) {
+    const editButton = page.locator('a[href^="/guide/editall/"]');
+    await delay(500);
+    await editButton.click();
+    await delay(500);
+}
+
+
+test.beforeEach(async ({ page }, testInfo) => {
+    captureConsoleMessages(page);
+    testInfo.setTimeout(90000);
+
+    // Login before running each test.
+    await login(page);
+});
+
+test.afterEach(async ({ page }) => {
+    // Logout after running each test.
+    await logout(page);
+});
+
+
+test('editall page', async ({ page }) => {
+    await createNewFeature(page);
+    await gotoEditAllPage(page);
+
+    await expect(page).toHaveScreenshot('edit-all-fields.png');
+});
+
+
+test('test semantic checks', async ({ page }) => {
+    await createNewFeature(page);
+    await gotoEditAllPage(page);
+
+    const devTrialDesktopInput = page.locator('input[name="dt_milestone_desktop_start"]');
+    await devTrialDesktopInput.fill('100');
+    await devTrialDesktopInput.blur(); // Must blur to trigger change event.
+    await delay(500);
+
+    // Check that there is no error now for the dev trail milestone field
+    const devTrailDesktopMilestoneLocator = page.locator('chromedash-form-field[name="dt_milestone_desktop_start"]');
+    await expect(devTrailDesktopMilestoneLocator.locator('.check-warning')).toHaveCount(0);
+
+    // Enter shipped desktop milestone of same number
+    const shippedDesktopInput = page.locator('input[name="shipped_milestone"]');
+    await shippedDesktopInput.fill('100');
+    await shippedDesktopInput.blur(); // Must blur to trigger change event.
+    await delay(500);
+
+    // Scroll next field into view, so we can see the error.
+    const shippedAndroidInput = page.locator('input[name="shipped_android_milestone"]');
+    await shippedAndroidInput.scrollIntoViewIfNeeded();
+    await delay(500);
+
+    // Test that the error message is shown for invalid shipped date
+    await expect(page).toHaveScreenshot('shipped-desktop-error.png');
+
+    // Remove the cause of the error.
+    await shippedDesktopInput.fill('');
+    await shippedDesktopInput.blur(); // Must blur to trigger change event.
+    await delay(500);
+
+    // Check that there is no error now for the dev trail milestone field
+    const shippedDesktopMilestoneLocator = page.locator('chromedash-form-field[name="shipped_milestone"]');
+    await expect(shippedDesktopMilestoneLocator.locator('.check-warning')).toHaveCount(0);
+});

packages/playwright/tests/chromedash-guide-feature-page_pwtest.js

@@ -0,0 +1,75 @@
+// @ts-check
+import { test, expect } from '@playwright/test';
+import {
+    captureConsoleMessages, delay, login, logout,
+    gotoNewFeaturePage, enterBlinkComponent, createNewFeature
+} from './test_utils';
+
+
+test.beforeEach(async ({ page }, testInfo) => {
+    captureConsoleMessages(page);
+    testInfo.setTimeout(90000);
+
+    // Login before running each test.
+    await login(page);
+});
+
+test.afterEach(async ({ page }) => {
+    // Logout after running each test.
+    await logout(page);
+});
+
+
+test('add an origin trial stage', async ({ page }) => {
+    // Safest way to work with a unique feature is to create it.
+    await createNewFeature(page);
+
+    // Add an origin trial stage.
+    const addStageButton = page.getByText('Add stage');
+    await addStageButton.click();
+    await delay(500);
+
+    // Select stage to create
+    const stageSelect = page.locator('sl-select#stage_create_select');
+    await stageSelect.click();
+    await delay(500);
+
+    // Hover Origin trial stage option.
+    const originTrialStageOption = page.locator('sl-select sl-option[value="150"]');
+    await originTrialStageOption.hover();
+    await delay(500);
+
+    // Screenshot of this dialog.
+    await expect(page).toHaveScreenshot('create-origin-trial-stage-dialog.png', {
+        mask: [page.locator('section[id="history"]')]
+    });
+
+    // Click the origin trial stage option to prepare to create stage.
+    originTrialStageOption.click();
+    await delay(500);
+
+    // Click the Create stage button to finally create the stage.
+    const createStageButton = page.getByText('Create stage');
+    await createStageButton.click();
+    await delay(500);
+
+    // Check we are still on the feature page.
+    await page.waitForURL('**/feature/*', { timeout: 5000 });
+    await delay(500);
+
+    // Expand the "Origin Trial" and "Origin Trial 2" panels
+    const originTrialPanel = page.locator('sl-details[summary="Origin Trial"]');
+    const originTrial2Panel = page.locator('sl-details[summary="Origin Trial 2"]');
+    await originTrialPanel.click();
+    await originTrial2Panel.click();
+    await delay(500);
+
+    // Take a screenshot of the content area.
+    // First scroll to "Prepare to ship" panel
+    const prepareToShipPanel = page.getByText('Prepare to ship');
+    await prepareToShipPanel.scrollIntoViewIfNeeded();
+    await delay(500);
+    await expect(page).toHaveScreenshot('origin-trial-panels.png', {
+        mask: [page.locator('section[id="history"]')]
+    });
+});

packages/playwright/tests/chromedash-guide-new-page_pwtest.js

@@ -1,11 +1,14 @@
 // @ts-check
 import { test, expect } from '@playwright/test';
-import { captureConsoleMessages, isMobile, delay, login, logout } from './test_utils';
+import {
+  captureConsoleMessages, delay, login, logout,
+  gotoNewFeaturePage, enterBlinkComponent, createNewFeature
+} from './test_utils';
 
 
-test.beforeEach(async ({page}) => {
+test.beforeEach(async ({ page }, testInfo) => {
   captureConsoleMessages(page);
-  test.setTimeout(90000);
+  testInfo.setTimeout(90000);
 
   // Login before running each test.
   await login(page);
@@ -17,33 +20,6 @@ test.afterEach(async ({ page }) => {
 });
 
 
-/**
- * @param {import('@playwright/test').Page} page
- */
-async function gotoNewFeaturePage(page) {
-  // console.log('navigate to create feature page');
-  const mobile = await isMobile(page);
-  const createFeatureButton = page.getByTestId('create-feature-button');
-  const menuButton = page.locator('[data-testid=menu]');
-
-  // Navigate to the new feature page.
-  await expect(menuButton).toBeVisible();
-  if (mobile) {
-    await menuButton.click();  // To show menu.
-  }
-  await createFeatureButton.click();
-  if (mobile) {
-    await menuButton.click();  // To hide menu
-    await delay(500);
-  }
-
-  // Expect "Add a feature" header to be present.
-  const addAFeatureHeader = page.getByTestId('add-a-feature');
-  await expect(addAFeatureHeader).toBeVisible({ timeout: 10000 });
-  // console.log('navigate to create feature page done');
-}
-
-
 test('navigate to create feature page', async ({page}) => {
   await gotoNewFeaturePage(page);
 
@@ -69,6 +45,46 @@ test('enter feature name', async ({page}) => {
   await delay(500);
 
   await expect(page).toHaveScreenshot('feature-name.png');
+
+});
+
+
+test('test semantic checks', async ({ page }) => {
+  await gotoNewFeaturePage(page);
+
+  // Enter feature name
+  const featureNameInput = page.locator('input[name="name"]');
+  await featureNameInput.fill('Test deprecated feature name');
+  await delay(500);
+
+  // Enter summary description
+  const summaryInput = page.locator('textarea[name="summary"]');
+  await summaryInput.fill('Test summary description');
+  await summaryInput.blur(); // Must blur to trigger change event.
+  await delay(500);
+
+  // Check that the warning shows up.
+  const summaryLocator = page.locator('chromedash-form-field[name="summary"]');
+  await expect(summaryLocator).toContainText('Feature summary should be');
+
+  // Screenshot of warnings about feature name summary length
+  await expect(page).toHaveScreenshot('warning-feature-name-and-summary-length.png', {
+    mask: [page.locator('section[id="history"]')]
+  });
+  await delay(500);
+
+  // Fix cause of the error
+  await summaryInput.fill('0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789');
+  await summaryInput.blur(); // Must blur to trigger change event.
+
+  // Check that there is no error now for the summary field
+  await expect(summaryLocator).not.toContainText('Feature summary should be');
+
+  // // Save changes
+  // const submitButton = page.locator('input[type="submit"]');
+  // await expect(submitButton).toBeVisible();
+  // await submitButton.click();
+  // await delay(500);
 });
 
 
@@ -80,51 +96,14 @@ test('enter blink component', async ({ page }) => {
   await blinkComponentsField.scrollIntoViewIfNeeded();
   await expect(blinkComponentsField).toBeVisible();
 
-  const blinkComponentsInputWrapper = page.locator('div.datalist-input-wrapper');
-  await expect(blinkComponentsInputWrapper).toBeVisible();
-
-  // Trying to show options, doesn't work yet.
-  await blinkComponentsInputWrapper.focus();
-  await delay(500);
-
-  const blinkComponentsInput = blinkComponentsInputWrapper.locator('input');
-  await blinkComponentsInput.fill('blink');
-  await delay(500);
+  await enterBlinkComponent(page);
 
   await expect(page).toHaveScreenshot('blink-components.png');
 });
 
 
 test('create new feature', async ({ page }) => {
-  await gotoNewFeaturePage(page);
-
-  // Enter feature name
-  const featureNameInput = page.locator('input[name="name"]');
-  await featureNameInput.fill('Test feature name');
-  await delay(500);
-
-  // Enter summary description
-  const summaryInput = page.locator('textarea[name="summary"]');
-  await summaryInput.fill('Test summary description');
-  await delay(500);
-
-  // Select blink component.
-  const blinkComponentsInputWrapper = page.locator('div.datalist-input-wrapper');
-  await blinkComponentsInputWrapper.focus();
-  await delay(500);
-  const blinkComponentsInput = blinkComponentsInputWrapper.locator('input');
-  await blinkComponentsInput.fill('blink');
-  await delay(500);
-
-  // Select feature type.
-  const featureTypeRadioNew = page.locator('input[name="feature_type"][value="0"]');
-  await featureTypeRadioNew.click();
-  await delay(500);
-
-  // Submit the form.
-  const submitButton = page.locator('input[type="submit"]');
-  await submitButton.click();
-  await delay(500);
+  await createNewFeature(page);
 
   // Screenshot of this new feature.
   await expect(page).toHaveScreenshot('new-feature-created.png', {
@@ -132,26 +111,5 @@ test('create new feature', async ({ page }) => {
   });
   await delay(500);
 
-  // Edit the feature.
-  const editButton = page.locator('a[class="editfeature"]');
-  await editButton.click();
-  await delay(500);
-
-  // Screenshot editor page
-  await expect(page).toHaveScreenshot('new-feature-edit.png');
-
-  // The following causes flakey errors.
-  // // Register to accept the confirm dialog before clicking to delete.
-  // page.once('dialog', dialog => dialog.accept());
-
-  //   // Delete the new feature.
-  // const deleteButton = page.locator('a[id$="delete-feature"]');
-  // await deleteButton.click();
-  // await delay(500);
-
-  // Screenshot the feature list after deletion.
-  // Not yet, since deletion only marks the feature as deleted,
-  // and the resulting page is always different.
-  // await expect(page).toHaveScreenshot('new-feature-deleted.png');
-  // await delay(500);
+  // await deleteFeature(page);
 });

packages/playwright/tests/chromedash-guide-stage-page_pwtest.js

@@ -0,0 +1,154 @@
+// @ts-check
+import { test, expect } from '@playwright/test';
+import {
+    captureConsoleMessages, delay, login, logout,
+    gotoNewFeaturePage, enterBlinkComponent, createNewFeature
+} from './test_utils';
+
+
+test.beforeEach(async ({ page }, testInfo) => {
+    captureConsoleMessages(page);
+    testInfo.setTimeout(90000);
+
+    // Login before running each test.
+    await login(page);
+});
+
+test.afterEach(async ({ page }) => {
+    // Logout after running each test.
+    await logout(page);
+});
+
+
+test('edit origin trial stage', async ({ page }) => {
+    // Safest way to work with a unique feature is to create it.
+    await createNewFeature(page);
+
+    // Add an origin trial stage.
+    const addStageButton = page.getByText('Add stage');
+    await addStageButton.click();
+    await delay(500);
+
+    // Select stage to create
+    const stageSelect = page.locator('sl-select#stage_create_select');
+    await stageSelect.click();
+    await delay(500);
+
+    // Click the origin trial stage option to prepare to create stage.
+    const originTrialStageOption = page.locator('sl-select sl-option[value="150"]');
+    originTrialStageOption.click();
+    await delay(500);
+
+    // Click the Create stage button to finally create the stage.
+    const createStageButton = page.getByText('Create stage');
+    await createStageButton.click();
+    await delay(500);
+
+    // Edit the Origin Trial (1) stage
+    const originTrialPanel = page.locator('sl-details[summary="Origin Trial"]');
+    await originTrialPanel.click();
+    const editFieldsButton = originTrialPanel.locator('sl-button[href^="/guide/stage"]');
+    await editFieldsButton.click();
+    await delay(500);
+
+    await page.waitForURL('**/guide/stage/*/*/*');
+
+    // Find the desktop start milestone field
+    const originTrialDesktopInput = page.locator('input[name="ot_milestone_desktop_start"]');
+    await originTrialDesktopInput.fill('100');
+    await originTrialDesktopInput.blur(); // Must blur to trigger change event.
+    await delay(500);
+
+    // Enter the same value for the _end field
+    const originTrialDesktopEndInput = page.locator('input[name="ot_milestone_desktop_end"]');
+    await originTrialDesktopEndInput.fill('100');
+    await originTrialDesktopEndInput.blur(); // Must blur to trigger change event.
+    await delay(500);
+
+    // Check that there is an error now for the origin trail milestone fields
+    const originTrailDesktopMilestoneStartLocator = page.locator('chromedash-form-field[name="ot_milestone_desktop_start"]');
+    await expect(originTrailDesktopMilestoneStartLocator.locator('.check-error')).toHaveCount(1);
+
+    const originTrailDesktopMilestoneEndLocator = page.locator('chromedash-form-field[name="ot_milestone_desktop_end"]');
+    await expect(originTrailDesktopMilestoneEndLocator.locator('.check-error')).toHaveCount(1);
+
+    // Scroll to a later field to center the OT milestone fields.
+    const originTrialAndroidMilestoneStart =
+        page.locator('chromedash-form-field[name="ot_milestone_android_start"]');
+    await originTrialAndroidMilestoneStart.scrollIntoViewIfNeeded();
+    await delay(500);
+
+    // Screenshot
+    await expect(page).toHaveScreenshot('semantic-check-origin-trial.png');
+
+    // Remove the end value
+    await originTrialDesktopEndInput.fill('');
+    await originTrialDesktopEndInput.blur(); // Must blur to trigger change event.
+    await delay(500);
+
+    // Check that there is no error now.
+    await expect(originTrailDesktopMilestoneStartLocator.locator('.check-error')).toHaveCount(0);
+    await expect(originTrailDesktopMilestoneEndLocator.locator('.check-error')).toHaveCount(0);
+
+    // Get the Submit button, to submit the change of OT start milestone.
+    const submitButton = page.locator('input[type="submit"]');
+    await submitButton.click();
+    await delay(500);
+
+    // Check that we are back on the Feature page
+    await page.waitForURL('**/feature/*');
+
+    // Edit the Origin Trial (2) stage
+    const originTrial2Panel = page.locator('sl-details[summary="Origin Trial 2"]');
+    await originTrial2Panel.click();
+    const editFieldsButton2 = originTrial2Panel.locator('sl-button[href^="/guide/stage"]');
+    await editFieldsButton2.click();
+    await delay(500);
+    await page.waitForURL('**/guide/stage/*/*/*');
+
+    // Find the desktop end milestone field
+    const originTrial2DesktopInput = page.locator('input[name="ot_milestone_desktop_end"]');
+    await originTrial2DesktopInput.fill('100');
+    await originTrial2DesktopInput.blur();  // To trigger change event.
+
+    // Check that there is no error.
+    const originTrail2DesktopMilestoneEndLocator = page.locator('chromedash-form-field[name="ot_milestone_desktop_end"]');
+    await expect(originTrail2DesktopMilestoneEndLocator.locator('.check-error')).toHaveCount(0);
+
+    // Submit this change
+    const submitButton2 = page.locator('input[type="submit"]');
+    await submitButton2.click();
+    await delay(500);
+
+    // Wait until we are back on the feature page
+    await page.waitForURL('**/feature/*');
+
+    // Open the Prepare to ship section.
+    const prepareToShipPanel = page.locator('sl-details[summary="Prepare to ship"]');
+    await prepareToShipPanel.click();
+    await delay(500);
+
+    // click 'Edit fields' button to go to the stage page.
+    const editFieldsButton3 = prepareToShipPanel.locator('sl-button[href^="/guide/stage"]');
+    await editFieldsButton3.click();
+    await delay(500);
+
+    // Find the shipped_milestone field
+    const shippedMilestoneInput = page.locator('input[name="shipped_milestone"]');
+    // Enter the same milestone as the OT 1 start.
+    await shippedMilestoneInput.fill('100');
+    await shippedMilestoneInput.blur();  // To trigger change event.
+    await delay(500);
+
+    // Check that there is a warning message.
+    const shippedMilestoneLocator = page.locator('chromedash-form-field[name="shipped_milestone"]');
+    await expect(shippedMilestoneLocator).toContainText('All origin trials starting milestones should be before feature shipping milestone.');
+
+    // Warning should allow submit
+    const submitButton3 = page.locator('input[type="submit"]');
+    await submitButton3.click();
+    await delay(500);
+
+    // We should be back on the feature page.
+    await page.waitForURL('**/feature/*');
+});

packages/playwright/tests/test_utils.js

@@ -1,5 +1,6 @@
 // @ts-check
 import { expect } from '@playwright/test';
+import { Page } from "playwright-core";
 
 
 /**
@@ -13,7 +14,7 @@ export async function delay(ms) {
 /**
  * Call this, say in your test.beforeEach() method, to capture all
  * console messages and copy them to the playwright console.
- * @param {import("playwright-core").Page} page
+ * @param {Page} page
  */
 export function captureConsoleMessages(page) {
   page.on('console', async msg => {
@@ -52,7 +53,7 @@ export function captureConsoleMessages(page) {
 }
 
 /**
- * @param {import("playwright-core").Page} page
+ * @param {Page} page
  */
 export function capturePageEvents(page) {
   // page.on('open', async () => {
@@ -87,7 +88,7 @@ export function capturePageEvents(page) {
 }
 
 /**
- * @param {import("playwright-core").Page} page
+ * @param {Page} page
  */
 export async function decodeCookies(page) {
   const cookies = await page.context().cookies();
@@ -97,13 +98,53 @@ export async function decodeCookies(page) {
 }
 
 /**
- * @param {import("playwright-core").Page} page
+ * @param {Page} page
  */
 export async function isMobile(page) {
   const viewportSize = page.viewportSize();
   return (viewportSize && viewportSize.width <= 700)
 }
 
+
+/**
+ * Handle beforeunload by accepting it.
+ * @param {import('@playwright/test').Page} page
+ */
+export function acceptBeforeUnloadDialogs(page) {
+  page.on('dialog', async dialog => {
+    if (dialog.type() === 'beforeunload') {
+      await dialog.accept();
+    }
+  });
+}
+
+/**
+ * Handle confirm dialog by accepting it.
+ * @param {import('@playwright/test').Page} page
+ */
+export function acceptConfirmDialogs(page) {
+  // Setup handler for confirm dialog
+  page.on('dialog', async dialog => {
+    if (dialog.type() === 'confirm') {
+      await dialog.accept();
+    }
+  });
+}
+
+/**
+ * Handle alert dialog by accepting it.
+ * @param {import('@playwright/test').Page} page
+ */
+export function acceptAlertDialogs(page) {
+  // Setup handler for confirm dialog
+  page.on('dialog', async dialog => {
+    if (dialog.type() === 'alert') {
+      await dialog.accept();
+    }
+  });
+}
+
+
 // Timeout for logging in, in milliseconds.
 // Initially set to longer timeout, in case server needs to warm up and
 // respond to the login.  Changed to shorter timeout after login is successful.
@@ -111,13 +152,17 @@ export async function isMobile(page) {
 let loginTimeout = 20000;
 
 /**
- * @param {import("playwright-core").Page} page
+ * @param {Page} page
  */
 export async function login(page) {
 
   page.exposeFunction('isPlaywright', () => {});
 
   // Always reset to the roadmap page.
+  // But first accept alert dialogs, which
+  // can occur in Chrome when not logged in.
+  acceptAlertDialogs(page);
+
   await page.pause();
   // console.log('login: goto /');
   await page.goto('/', {timeout: 20000});
@@ -168,17 +213,22 @@ export async function login(page) {
 }
 
 /**
- * @param {import("playwright-core").Page} page
+ * @param {Page} page
  */
 export async function logout(page) {
   // Attempt to sign out after running each test.
-  // First reset to the roadmap page.
+  // First reset to the roadmap page, so that we avoid the alert
+  // when signed out on other pages.
+  // But in case the current page has unsaved changes we need to
+  // accept leaving them unsaved.
+  acceptBeforeUnloadDialogs(page);
+
   // console.log('logout: goto /');
   await page.goto('/');
   await page.waitForURL('**/roadmap');
   await delay(1000);
-
   await expect(page).toHaveTitle(/Chrome Status/);
+
   page.mouse.move(0, 0); // Move away from content on page.
   await delay(1000);
 
@@ -196,10 +246,128 @@ export async function logout(page) {
   // Need to hover to see the sign-out-link
   const signOutLink = page.getByTestId('sign-out-link');
   await expect(signOutLink).toBeVisible();
-  await signOutLink.click({timeout: 5000});
+  await signOutLink.click({ timeout: 5000 });
+  await delay(500);
 
   await page.waitForURL('**/roadmap');
   await expect(page).toHaveTitle(/Chrome Status/);
 
+  // Redundant? Go to roadmap page.
+  await page.goto('/');
+  await page.waitForURL('**/roadmap');
+  await delay(500);
+
   // console.log('logout: done');
 }
+
+
+/**
+ * From top-level page, after logging in, go to the New Feature page.
+ * @param {Page} page
+ */
+export async function gotoNewFeaturePage(page) {
+  // console.log('navigate to create feature page');
+  const mobile = await isMobile(page);
+  const createFeatureButton = page.getByTestId('create-feature-button');
+  const menuButton = page.locator('[data-testid=menu]');
+
+  // Navigate to the new feature page.
+  await expect(menuButton).toBeVisible();
+  if (mobile) {
+    await menuButton.click();  // To show menu.
+  }
+  await createFeatureButton.click();
+  if (mobile) {
+    await menuButton.click();  // To hide menu
+    await delay(500);
+  }
+
+  // Expect "Add a feature" header to be present.
+  const addAFeatureHeader = page.getByTestId('add-a-feature');
+  await expect(addAFeatureHeader).toBeVisible({ timeout: 10000 });
+  // console.log('navigate to create feature page done');
+  await delay(500);
+}
+
+
+/**
+ * Enters a blink component on the page.
+ *
+ * @param {Page} page - The page object representing the web page.
+ * @return {Promise<void>} A promise that resolves once the blink component is entered.
+ */
+export async function enterBlinkComponent(page) {
+  const blinkComponentsInputWrapper = page.locator('div.datalist-input-wrapper');
+  await expect(blinkComponentsInputWrapper).toBeVisible();
+
+  // Trying to show options, doesn't work yet.
+  await blinkComponentsInputWrapper.focus();
+  await delay(500);
+
+  const blinkComponentsInput = blinkComponentsInputWrapper.locator('input');
+  await blinkComponentsInput.fill('blink');
+  await delay(500);
+}
+
+/**
+ * Create a new feature, starting from top-level page, ending up on feature page.
+ * @param {import('@playwright/test').Page} page
+ */
+export async function createNewFeature(page) {
+  await gotoNewFeaturePage(page);
+
+  // Enter feature name
+  const featureNameInput = page.locator('input[name="name"]');
+  await featureNameInput.fill('Test feature name');
+  await delay(500);
+
+  // Enter summary description
+  const summaryInput = page.locator('textarea[name="summary"]');
+  await summaryInput.fill('Test summary description');
+  await delay(500);
+
+  await enterBlinkComponent(page);
+
+  // Select feature type.
+  const featureTypeRadioNew = page.locator('input[name="feature_type"][value="0"]');
+  await featureTypeRadioNew.click();
+  await delay(500);
+
+  // Submit the form.
+  const submitButton = page.locator('input[type="submit"]');
+  await submitButton.click();
+  await delay(500);
+
+  // Wait until we are on the Feature page.
+  await page.waitForURL('**/feature/*');
+  await delay(500);
+}
+
+
+/**
+ * Starting from the feature page, edit the feature
+ * @param {import('@playwright/test').Page} page
+ */
+export async function editFeature(page) {
+  // Edit the feature.
+  const editButton = page.locator('a.editfeature');
+  await delay(500);
+  await editButton.click();
+  await delay(500);
+
+  await page.waitForURL('**/guide/edit/*');
+  await delay(500);
+}
+
+
+/**
+ * Starting from the feature page, delete the feature
+ * @param {import('@playwright/test').Page} page
+ */
+export async function deleteFeature(page) {
+  await editFeature(page);
+
+  const deleteButton = page.locator('#delete-feature');
+  await deleteButton.click();
+  await delay(500);
+}

playwright.config.cjs

@@ -1,85 +0,0 @@
-// @ts-check
-const { defineConfig, devices } = require('@playwright/test');
-
-/**
- * Read environment variables from file.
- * https://github.com/motdotla/dotenv
- */
-// require('dotenv').config();
-
-/**
- * @see https://playwright.dev/docs/test-configuration
- */
-module.exports = defineConfig({
-  // Directory where the tests are located. "." for top-level directory.
-  testDir: '.',
-  // Glob patterns or regular expressions that match test files.
-  testMatch: '*/*_pwtest.js',
-  snapshotPathTemplate: '{testDir}/{testFileDir}/__screenshots__/{testFileName}/{testName}-{arg}-{projectName}-{platform}{ext}',
-
-  /* Run tests in files in parallel */
-  fullyParallel: true,
-  /* Fail the build on CI if you accidentally left test.only in the source code. */
-  forbidOnly: !!process.env.CI,
-  /* Retry on CI only */
-  retries: 0, // process.env.CI ? 2 : 0,
-  /* Opt out of parallel tests. */
-  workers: 1, // process.env.CI ? 1 : undefined,
-  /* Reporter to use. Not for CI. See https://playwright.dev/docs/test-reporters */
-  reporter: process.env.CI ? [ ['html', { open: 'never'}] ] : [ ['html', { open: 'always'}] ],
-  /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */
-  use: {
-    /* Base URL to use in actions like `await page.goto('/')`. */
-    baseURL: 'http://localhost:8080',
-
-    /* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */
-    trace: 'on-first-retry',
-  },
-  preserveOutput: 'always',
-
-  /* Configure projects for major browsers */
-  projects: [
-    {
-      name: 'chromium',
-      use: { ...devices['Desktop Chrome'] },
-    },
-
-    {
-      name: 'firefox',
-      use: { ...devices['Desktop Firefox'] },
-    },
-
-    // Dependencies for webkit are not working yet.
-    // {
-    //   name: 'webkit',
-    //   use: { ...devices['Desktop Safari'] },
-    // },
-
-    /* Test against mobile viewports. */
-    // {
-    //   name: 'Mobile Chrome',
-    //   use: { ...devices['Pixel 5'] },
-    // },
-    // {
-    //   name: 'Mobile Safari',
-    //   use: { ...devices['iPhone 12'] },
-    // },
-
-    /* Test against branded browsers. */
-    // {
-    //   name: 'Microsoft Edge',
-    //   use: { ...devices['Desktop Edge'], channel: 'msedge' },
-    // },
-    // {
-    //   name: 'Google Chrome',
-    //   use: { ..devices['Desktop Chrome'], channel: 'chrome' },
-    // },
-  ],
-
-  /* Run your local dev server before starting the tests */
-  webServer: {
-    command: 'npm run start',
-    url: 'http://localhost:8080',
-    reuseExistingServer: true,
-  },
-});