Some reason after bug 1391293, some urls get cropped really early when they would fit fine. See attachment in particular the 5th result that the arrow line points to. This only happens when results update. I.e., if I paste in "fx57", the results are fine. It only causes problems when I type out "f" "x" "5" "7". My guess is that on the 5th line, the previous result used to have a long page title resulting in a cropped url. Then when the new 5th line result came in, the url did not uncrop.

We must not be calling handleOverUnderflow or handleOverflow on the reused item. Maybe not even _reuseAcItem? I'd need to look more to see, and to see what we should be calling, due to bug 1391293.

In bug 1391293 (Second part) I indeed removed lots of calls to handleOverUnderflow when we reuse items, because I wrongly assumed we were always cropping at the end. Sounds like I was wrong, we also crop titles. We likely need to make my optimization less aggressive. Though that means reintroducing lots of potential reflows.

We may likely annotate somewhere if the title match was cropped and recalculate if true? (didn't actually check code, just guessing)

FWIW I'm pretty sure I didn't search for anything to have the bug appear as per bug 1402281. The order and position of the suggestions are exactly the same as when I just click the down arrow to open the list. Also, not only the url, but also the title was affected when the bug was present.

cers, did you ever type anything in the address bar? In my current window, I also do see titles being cropped when I click the down arrow. But if I open a new window, the down arrow shows full titles and urls. So definitely seems like an issue when autocomplete results were previously sized for some old result and used for new results. In any case, per comment 2, sounds like there are initial ideas on what's causing this and ways to fix.

hmm, I wonder if it's related to this in _adjustAcItem(): if (!popup.popupOpen) { // Removing the max-width and resetting it later when overflow is // handled is jarring when the item is visible, so skip this when // the popup is open. this._removeMaxWidths(); } In particular, it's possible now we update results more often while the popup is open and maybe we didn't notice the problem before. Btw, I'm investigating this.

I just tried bypassing the if in comment 7, and it seems to solve the bug.

Comment on attachment 8911245 [details] Bug 1402178 - Address bar title and urls get cropped at strange places when matches are reused. https://reviewboard.mozilla.org/r/182740/#review188072 r+ because the general idea makes sense and we want to fix this ASAP, but I have one major question. r+ with that fixed/addressed/explained. ::: toolkit/content/widgets/autocomplete.xml:2355 (Diff revision 2) > } > this._setUpDescription(this._urlText, displayUrl, !emphasiseUrl); > > - if (this._inOverflow) { > + // Removing the max-width may be jarring when the item is visible, but > + // we have no choice to properly crop the text. > + // This may cause the overflow event. Nit: This comment's formatting is strange. The second line is unnecessarily short. ::: toolkit/content/widgets/autocomplete.xml:2358 (Diff revision 2) > - if (this._inOverflow) { > + // Removing the max-width may be jarring when the item is visible, but > + // we have no choice to properly crop the text. > + // This may cause the overflow event. > + let wasInOverflow = this._inOverflow; > + this._removeMaxWidths(); > + if (wasInOverflow && this._inOverflow) { Checking wasInOverflow doesn't seem right. If !wasInOverflow, but now _inOverflow, we need to handle the overflow, right? IOW _inOverflow is the only relevant condition. But actually, if !wasInOverflow && _inOverflow, then doesn't that mean that _onOverflow was called (as a result of removing max widths)? And since _onOverflow calls _handleOverflow, it doesn't seem like you need to do this at all. This would end up calling _handleOverflow an unnecessary second time it seems to me. ::: toolkit/content/widgets/autocomplete.xml:2473 (Diff revision 2) > tagsAvailable, > titleTagsMaxWidth * (1 - titlePct) > ); > this._titleText.style.maxWidth = titleMaxWidth + "px"; > this._tagsText.style.maxWidth = tagsMaxWidth + "px"; > + this._hasMaxWidths = true; Unless I'm reading this wrong, this setter isn't necessary because the other one you added below will always be hit when this one is, right?

(In reply to Drew Willcoxon :adw from comment #11) > Checking wasInOverflow doesn't seem right. If !wasInOverflow, but now > _inOverflow, we need to handle the overflow, right? IOW _inOverflow is the > only relevant condition. > > But actually, if !wasInOverflow && _inOverflow, then doesn't that mean that > _onOverflow was called (as a result of removing max widths)? And since > _onOverflow calls _handleOverflow, it doesn't seem like you need to do this > at all. This would end up calling _handleOverflow an unnecessary second > time it seems to me. I thought the same, but if I remove the _handleOverflow call, sometimes the crop is wrong. We may be in a case where we are overflowed, we replace the text, the new text is also overflowed, so inOverflow is true before AND after we remove max widths and we get no overflow event.

To be clear, we may be replacing overflowed text with other overflowed text, but different one, so that before we could have cropped the title, while now we crop the url.

[Tracking Requested - why for this release]: 57 regression in primary ui.

Pushed by mak77@bonardo.net: https://hg.mozilla.org/integration/autoland/rev/c4252999e145 Address bar title and urls get cropped at strange places when matches are reused. r=adw

Comment on attachment 8911245 [details] Bug 1402178 - Address bar title and urls get cropped at strange places when matches are reused. Approval Request Comment [Feature/Bug causing the regression]: bug 1391293 [User impact if declined]: Some results in the Address Bar are cut with ellipsis at the wrong place, making them sometimes unreadable/useless. [Is this code covered by automated tests?]: no, it's not trivial to make a good one [Has the fix been verified in Nightly?]: not yet [Needs manual test from QE? If yes, steps to reproduce]: I think it's obvious enough to see for users that I'd not spend QE time on it, we'll hear back soon if something's wrong yet [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: the only touched code is the string cropping one, so at a maximum it would cause this same bug [String changes made/needed]: none

Looks fixed for me on 20170926100259. Tested with all my single letter autocompletes on a narrow window that had been showing the random croppings.

Comment on attachment 8911245 [details] Bug 1402178 - Address bar title and urls get cropped at strange places when matches are reused. Fix a regression, taking it. Should be in 57b4 (gtb tomorrow Thursday)