webdino / amethyst

Simple WebViewer App for embedded systems (patches for Firefox/Gecko)
https://gecko-embedded.org/docs/webviewer/
7 stars 1 forks source link

フォームバリデーションによるエラーメッセージが表示されない #37

Closed dynamis closed 5 years ago

dynamis commented 5 years ago

フォーム機能のうち、 ポップアップなどの表示を伴う機能が正常に動作していない。

ポップアップ表示でエラーを表示するところで browser.xul 依存があり失敗してブロックせずに素通ししているのか、単純に validation 機能自体が動作してないのか確認するところから。いずれにしても validation 出来てないのは不味い。

テストページ

ピッカー系のウィンドウを開くと困る件は Firefox と共通の別件: https://github.com/webdino/gecko-embedded/issues/78

参考

dynamis commented 5 years ago

バリデーションエラーのまま送信できてしまっているケースがあると思ったがどうやら勘違いで最新のビルドでは一通り問題なかった。但し、Firefox が対応していないバリデーション指定については除く (未対応のバリデーションを通ってしまったことを勘違いしたかも)

バリデーションエラーのポップアップが出ないのはその表示に panel を使っているがそれが出てこないというエラーメッセージ this._panel is null が表示される。該当箇所はこちら:

https://dxr.mozilla.org/mozilla-esr60/source/browser/modules/FormValidationHandler.jsm#106

  _showPopup(aWindow, aPanelData) {
    let previouslyShown = !!this._panel;
    this._panel = aWindow.document.getElementById("invalid-form-popup");
    this._panel.firstChild.textContent = aPanelData.message;
    this._panel.hidden = false;

    let tabBrowser = aWindow.gBrowser;
    this._anchor = tabBrowser.popupAnchor;
    this._anchor.left = aPanelData.contentRect.left;
    this._anchor.top = aPanelData.contentRect.top;
    this._anchor.width = aPanelData.contentRect.width;
    this._anchor.height = aPanelData.contentRect.height;
    this._anchor.hidden = false;

    // Display the panel if it isn't already visible.
    if (!previouslyShown) {
      // Cleanup after the popup is hidden
      this._panel.addEventListener("popuphiding", this, true);

      // Hide if the user scrolls the page
      tabBrowser.selectedBrowser.addEventListener("scroll", this, true);
      tabBrowser.selectedBrowser.addEventListener("FullZoomChange", this);
      tabBrowser.selectedBrowser.addEventListener("TextZoomChange", this);
      tabBrowser.selectedBrowser.addEventListener("ZoomChangeUsingMouseWheel", this);

      // Open the popup
      this._panel.openPopup(this._anchor, aPanelData.position, 0, 0, false);
    }
  },
dynamis commented 5 years ago

このケースは比較的シンプルな対応で済みそう。browser.xul に panel 要素を入れてそのパネルの表示を切り替えるようにするのと、_showPopup の中で tabBrowser にアクセスしているところはせずに browser.xul の単一タブ固定処理で popupAnchor と同じことをするようにしておけば最小限のコードだけで全く同じ挙動が再現できそう (他に影響範囲がないことを祈る)。

https://dxr.mozilla.org/mozilla-esr60/source/browser/base/content/tabbrowser.js#259

  get popupAnchor() {
    if (this.mCurrentTab._popupAnchor) {
      return this.mCurrentTab._popupAnchor;
    }
    let stack = this.mCurrentBrowser.parentNode;
    // Create an anchor for the popup
    let popupAnchor = document.createElementNS(this._XUL_NS, "hbox");
    popupAnchor.className = "popup-anchor";
    popupAnchor.hidden = true;
    stack.appendChild(popupAnchor);
    return this.mCurrentTab._popupAnchor = popupAnchor;
  },

https://dxr.mozilla.org/mozilla-esr60/source/browser/base/content/browser.xul#192 https://dxr.mozilla.org/mozilla-esr60/source/browser/base/content/browser.css#959

dynamis commented 5 years ago

追記: 一応古いビルドを確認したらやはりフォームのバリデーションを通って submit できてしまっていた (特にコンソールへのエラーメッセージは無し)。WebRTC 向けの対応パッチなどの副作用でその問題も解消したっぽい?何故解消したのか詳細は追っていないが残るは上記のメッセージ表示をしたいところだけ (検証時には送信防止が動作しているかは要確認)

hATrayflood commented 5 years ago

WebRTCを有効にする方法として、content.jsを丸ごとの読み込んだ副作用だと思われます。 content.jsはWebRTC以外にもさまざまな機能を読み込んで有効にするコードがあるので、本プロジェクトの要件としては不要なものも含まれています。 必要最低限のものだけ読み込むよう直したうえで、あらためてこちらの問題も対応しようと思います。 @dynamis

dynamis commented 5 years ago

なるほど content.js 経由で FormSubmitObserver.jsm も読み込だからですね。すっきりしました。

https://dxr.mozilla.org/mozilla-esr60/source/browser/base/content/content.js#25

そして確かにそこから大量の jsm 読み込んでいて (Amethyst で省けるはずだった) オーバーヘッドが一部再発しているのは精査お願いします。

... 同じディレクトリに aboutRobots-icon.png とかあって懐かしかった

hATrayflood commented 5 years ago

@dynamis https://github.com/webdino/amethyst/commit/dc86152393f62b9a627bafa36dfe2b05ce7debd3 パッチ更新しました。これを踏まえてフォームバリデーション対応します。

... 同じディレクトリに aboutRobots-icon.png とかあって懐かしかった

ニンゲンノミナサン、ヨウコソ!

hATrayflood commented 5 years ago

https://github.com/webdino/amethyst/commit/4ebc9bb8de82883230abccb47b8d136b6401cb41 対応しました。ビルドと動作確認をお願いします。 @dynamis @YoshihiroOota @yoshikuni-kamimiya

kou029w commented 5 years ago

フォームバリデーション

https://quirksmode.org/html5/inputs/tests/inputs_text.html など

hATrayflood commented 5 years ago

https://github.com/webdino/amethyst/commit/99ffcbf5c5cbc8199b9dc1d688db21fb8cb89fc7 副作用により #36 でエラーが出るようになってしまったので、実装を変更しました。 @dynamis @kou029w

kou029w commented 5 years ago

190822_images_webviewer_11833efc.tar.gz 11833efc7e4295981cf352fdcddb5c5c8d471edc から disable-crashreporter.patch disable-telemetry.patch removed-files.patch を除外したビルドにて確認:

問題なく動作することを確認したためクローズします