910161 - Remove nsIHistoryEntry and replace it with nsISHEntry

Status: RESOLVED FIXED

(Core Graveyard :: History: Global, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

It seems like nsIHistoryEntry is a legacy interface that we actually don't use anymore. In most of the places it's QI'ed to nsISHEntry shortly after calling nsISHistory.GetEntryAtIndex().

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: remove nsIHistoryEntry

I have no clue about our interface retention policies and whether we can just remove nsIHistoryEntry? The patch does that.

If we can't remove the interface for backwards compatibility reasons, can we just make it an empty one extending nsISHEntry and mark it explicitly as something that's only around for bc?

I think that would reduce confusion when stumbling upon code that checks for and uses nsIHistoryEntry objects.

Comment 2

[Boris Zbarsky [:bzbarsky]]

https://mxr.mozilla.org/addons/search?string=nsihistoryentry&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=addons suggests some addons are using it.  Removing it would break them.

Making nsIHistoryEntry empty would work if all those addons are doing no-op QIs (as in they already have an nsIHistoryEntry or nsISHEntry object in JS).  On the other hand, if they're relying on the interface flattening behavior of QI it would break them.

Jorge, how much do we need to worry about preserving compat with this set of addons?

Comment 3

[Jorge Villalobos [:jorgev] (he/him)]

There are still a few of those that are still relevant (and Collusion is one of ours). If the fix on the add-on side is straightforward, like "replace nsIHistoryEntry with nsISHEntry", then it's fine to move forward without a compatibility shim. It's very easy for us to detect specific interface usage for our compatibility bumps, so we can notify the developers when the time is right.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 796564 [details] [diff] [review]
remove nsIHistoryEntry

In that case, this looks reasonable.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

https://tbpl.mozilla.org/?tree=Try&rev=650a62fbe6ec

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 796564 [details] [diff] [review]
remove nsIHistoryEntry

r=me

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

Thanks!

https://hg.mozilla.org/integration/fx-team/rev/7fbbfe346cad

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7fbbfe346cad