MaximBelov commented on PR #1073: URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/1073#issuecomment-5283612621
I built a test app for this and measured what the bar actually puts on screen. The result partly refutes what I argued above, so I would rather put it here myself. Repo: https://github.com/MaximBelov/cordova-iab-pr1073-progressbar-demo — it pins the pre-rewrite commit so it builds the bar, and includes the screenshots and the server used to produce them. Two load shapes, Android 15, fill measured by counting the bar's pixels rather than by eye: **Slow first byte** — the case #1075 and #1076 describe. The bar is up for the whole stall: present in 41 consecutive samples from t=0.77s to t=7.10s of a 7s server delay. But it reads exactly 10.0% in every one of them. It never moves. Chromium reports progress 10 while awaiting the document, then jumps to 100. **Fast document, slow subresources.** Here it does climb — 10% → 20% → 80% — but the whole sequence lasts about 400ms, and then it disappears while the page keeps loading for another ~12s. `onProgressChanged(100)` fires at document parse; pending `<img>` subresources do not hold it back. So my main argument does not survive: the bar is determinate in code, not on screen. In the case people actually complain about it holds a static 10% sliver for seven seconds, which reads as *stuck* rather than *working*, and conveys nothing your spinner wouldn't. That limitation belongs to the callback, not to either implementation — a spinner driven from the same callback behaves identically — but the spinner at least does not promise a number it cannot deliver. I withdraw the determinate-versus-indeterminate part. What I would still keep is narrower and untouched by the measurement: placement. A centred overlay reappears on top of content the user is already reading on every subsequent in-page navigation, where a strip under the toolbar does not. If you would rather stay with the centred spinner for iOS parity, I won't push further. The thing I care about either way is the hide condition. `progress == 100` is currently the only one. Reading the client, a load aborted before commit never reaches it: `onDownloadStart` for a `Content-Disposition: attachment` link, `onReceivedSslError` (the handler sends `loaderror` and the load is cancelled, so no error page loads), `shouldOverrideUrlLoading` returning true for a scheme handoff. Unlike the numbers above I have not measured those yet — I will run them on the same harness and report back rather than leave it as reasoning. If they hold, the fix is hiding in `onPageFinished`, `onReceivedError`, `onDownloadStart` and `onReceivedSslError`, which is what iOS already does in its didFail delegates, and it is the error handling #1076 explicitly asks for. Separately, on the black background you raised: https://github.com/MaximBelov/cordova-iab-pr1167-backgroundcolor-demo does the same for #1167. One thing worth knowing before you review it — the default follows the device theme, not the platform. In light mode the empty frame is already white; the no-option and `backgroundcolor=#FFFFFF` screenshots came out byte-identical. It is only black in dark mode. So #1167 looks like a no-op if it is tested in light mode. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
