Closed Bug 1406352 Opened 3 years ago Closed 3 years ago

Remove unused autocomplete-dropmarker.png

Categories

(Toolkit :: Themes, defect)

Unspecified
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: dao, Assigned: dao)

References

Details

Attachments

(1 file)

No description provided.
Why do you say it's unused? from the code it looks like it's overriding the default dropmarker on MacOS. Is that no more necessary with the recent OS updates?
(In reply to Marco Bonardo [::mak] from comment #2)
> Why do you say it's unused? from the code it looks like it's overriding the
> default dropmarker on MacOS.

Where is that used? There doesn't seem to be similar styling on Windows or Linux, and the address bar's dropmarker is styled in browser/themes/shared/urlbar-searchbar.inc.css.
the dropmarker in browser has both classes: autocomplete-history-dropmarker and urlbar-history-dropmarker. As such it sounds like it's also using rules like padding, margin, border and background-color that urlbar-searchbar.inc.css doesn't override.
Did you ensure these removed rules don't have effect on Mac?

autocomplete-history-dropmarker is also class for the dropmarker of the generic "autocomplete" biding:
http://searchfox.org/mozilla-central/rev/8efd128b48cdae3a6c6862795ce618aa1ca1c0b4/toolkit/content/widgets/autocomplete.xml#34
that is the base binding for textbox[type="autocomplete"]
Though, this seems to always display: none the history dropmarker:
http://searchfox.org/mozilla-central/rev/8efd128b48cdae3a6c6862795ce618aa1ca1c0b4/toolkit/content/xul.css#904
So this is probably a non-issue, since looks like only the urlbar unhides it. I wonder why it exists at all, since the urlbar overloads the binding contents... Probably just to complicate our life :)

So, I suppose you should just check on Mac that the removal of the rules doesn't break the urlbar visual design.
(In reply to Marco Bonardo [::mak] from comment #4)
> the dropmarker in browser has both classes: autocomplete-history-dropmarker
> and urlbar-history-dropmarker. As such it sounds like it's also using rules
> like padding, margin, border and background-color that
> urlbar-searchbar.inc.css doesn't override.
> Did you ensure these removed rules don't have effect on Mac?

padding is set here:

http://searchfox.org/mozilla-central/rev/8efd128b48cdae3a6c6862795ce618aa1ca1c0b4/browser/themes/shared/urlbar-searchbar.inc.css#173

and dropmarker.css doesn't set any default margin, border, or background-color:

http://searchfox.org/mozilla-central/source/toolkit/themes/osx/global/dropmarker.css

> So this is probably a non-issue, since looks like only the urlbar unhides
> it. I wonder why it exists at all, since the urlbar overloads the binding
> contents... Probably just to complicate our life :)

I can look into removing it in a followup.
Comment on attachment 8915946 [details]
Bug 1406352 - Remove unused autocomplete-dropmarker.png.

https://reviewboard.mozilla.org/r/187218/#review192308

r=me with a simple visual inspection of the location bar on Mac. That said, I assume we'd discover soon a visual regression there :)
Attachment #8915946 - Flags: review?(mak77) → review+
(In reply to Marco Bonardo [::mak] from comment #6)
> Comment on attachment 8915946 [details]
> Bug 1406352 - Remove unused autocomplete-dropmarker.png.
> 
> https://reviewboard.mozilla.org/r/187218/#review192308
> 
> r=me with a simple visual inspection of the location bar on Mac. That said,
> I assume we'd discover soon a visual regression there :)

I'm home now and don't have access to a Mac. I think I'll just land this -- based on code inspection, the removed styling is entirely redundant, and we can rely on nightly testing to catch any unforeseen fallout.
(In reply to Marco Bonardo [::mak] from comment #4)

> that is the base binding for textbox[type="autocomplete"]
> Though, this seems to always display: none the history dropmarker:
> http://searchfox.org/mozilla-central/rev/
> 8efd128b48cdae3a6c6862795ce618aa1ca1c0b4/toolkit/content/xul.css#904

Actually, when enablehistory="true" (http://searchfox.org/mozilla-central/rev/8efd128b48cdae3a6c6862795ce618aa1ca1c0b4/browser/base/content/browser.xul#786) the "display: none;" is overruled by this: http://searchfox.org/mozilla-central/rev/8efd128b48cdae3a6c6862795ce618aa1ca1c0b4/toolkit/content/xul.css#907
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s f4005c4b9b25 -d aa16d953827f: rebasing 424534:f4005c4b9b25 "Bug 1406352 - Remove unused autocomplete-dropmarker.png. r=mak" (tip)
merging toolkit/themes/osx/global/jar.mn
warning: conflicts while merging toolkit/themes/osx/global/jar.mn! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/05315cefb232
Remove unused autocomplete-dropmarker.png. r=mak
(In reply to Stefan [:stefanh] from comment #8)
> Actually, when enablehistory="true"
> (http://searchfox.org/mozilla-central/rev/
> 8efd128b48cdae3a6c6862795ce618aa1ca1c0b4/browser/base/content/browser.
> xul#786) the "display: none;" is overruled by this:
> http://searchfox.org/mozilla-central/rev/
> 8efd128b48cdae3a6c6862795ce618aa1ca1c0b4/toolkit/content/xul.css#907

Yes, and pretty much only the urlbar sets that attribute.
https://hg.mozilla.org/mozilla-central/rev/05315cefb232
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Blocks: 1407613
You need to log in before you can comment on or make changes to this bug.