Bug 8769 - [Model] is not propagated correctly on protocols
Summary: [Model] is not propagated correctly on protocols
Status: RESOLVED NOT_REPRODUCIBLE
Alias: None
Product: iOS
Classification: Xamarin
Component: Tools ()
Version: 6.0.x
Hardware: PC Mac OS
: --- normal
Target Milestone: Untriaged
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2012-12-05 08:10 UTC by Rolf Bjarne Kvinge [MSFT]
Modified: 2017-02-13 16:17 UTC (History)
7 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
Proposed patch (4.76 KB, patch)
2012-12-05 08:12 UTC, Rolf Bjarne Kvinge [MSFT]
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED NOT_REPRODUCIBLE

Description Rolf Bjarne Kvinge [MSFT] 2012-12-05 08:10:38 UTC
Context: http://forums.xamarin.com/discussion/comment/1647

[Model] information is lost when inlining protocols.

When the protocol contains optional methods, and the type with the inlined protocol does not actually implement some of those optional methods, we may end up throwing "selector not found" exceptions. Without the [Model] information we will export all the methods, even if they're not overridden, and automatically call the base implementation (which may not exist, thus the exception).

The forum thread has a sample where this occurs.
Comment 1 Rolf Bjarne Kvinge [MSFT] 2012-12-05 08:12:48 UTC
Created attachment 3059 [details]
Proposed patch

This patch will add a [Model] attribute to the inlined members.

MonoTouch will also have to be modified to actually take that [Model] attribute into account.
Comment 2 Rolf Bjarne Kvinge [MSFT] 2012-12-05 08:13:51 UTC
Miguel, does this look like a sane approach to the problem?
Comment 3 Miguel de Icaza [MSFT] 2012-12-05 20:54:05 UTC
[Model] was not supposed to be used for individual methods, only for classes.

So we need to discuss why this is needed, the patch alone is not enough for me to understand what we are trying to solve.

On the discussion linked, the attempt is being done to support something that we do not support at all.

That is, we do not support arbitrarily adding an interface, just because it claims that it adopts the protocol.

Let us discuss, but this is something that is explicitly not supported, and manual work arounds must be used, like manually inlining those methods.
Comment 4 Alex Soto 2012-12-05 22:51:48 UTC
But If we manual bind that, as I said that not the only class that implements the protocol even thats not the only protocol, that would be really ugly and hard to maintain.

For Example this Objective-C interface

@interface PSPDFSearchViewController : UITableViewController <UISearchDisplayDelegate, UISearchBarDelegate, PSPDFCacheDelegate, PSPDFTextSearchDelegate, PSPDFStatusBarStyleHint, PSPDFPopoverControllerDismissable>

My binding was

[BaseType (typeof (UITableViewController))]
interface PSPDFSearchViewController : UISearchDisplayDelegateProtocol, UISearchBarDelegateProtocol, PSPDFCacheDelegate, PSPDFTextSearchDelegate, PSPDFStatusBarStyleHint


I could not do : UISearchDisplayDelegateProtocol, UISearchDisplayDelegate which are part of MonoTouch so I had to take all methods of both UISearchDisplayDelegateProtocol and UISearchDisplayDelegate and create an interface without Export attribute on ApiDefinition.cs like this

interface UISearchDisplayDelegateProtocol 
	{
		[Export ("searchDisplayControllerWillBeginSearch:")]
		void SearchDisplayControllerWillBeginSearch (UISearchDisplayController controller);
		
		[Export ("searchDisplayControllerDidBeginSearch:")]
		void SearchDisplayControllerDidBeginSearch (UISearchDisplayController controller);
		
		[Export ("searchDisplayControllerWillEndSearch:")]
		void SearchDisplayControllerWillEndSearch (UISearchDisplayController controller);
		
		[Export ("searchDisplayControllerDidEndSearch:")]
		void SearchDisplayControllerDidEndSearch (UISearchDisplayController controller);
		
		[Export ("searchDisplayController:didLoadSearchResultsTableView:")]
		void SearchDisplayControllerDidLoadSearchResultsTableView (UISearchDisplayController controller, UITableView tableView);
		
		[Export ("searchDisplayController:willUnloadSearchResultsTableView:")]
		void SearchDisplayControllerWillUnloadSearchResultsTableView (UISearchDisplayController controller, UITableView tableView);
		
		[Export ("searchDisplayController:willShowSearchResultsTableView:")]
		void SearchDisplayControllerWillShowSearchResultsTableView (UISearchDisplayController controller, UITableView tableView);
		
		[Export ("searchDisplayController:didShowSearchResultsTableView:")]
		void SearchDisplayControllerDidShowSearchResultsTableView (UISearchDisplayController controller, UITableView tableView);
		
		[Export ("searchDisplayController:willHideSearchResultsTableView:")]
		void SearchDisplayControllerWillHideSearchResultsTableView (UISearchDisplayController controller, UITableView tableView);
		
		[Export ("searchDisplayController:didHideSearchResultsTableView:")]
		void SearchDisplayControllerDidHideSearchResultsTableView (UISearchDisplayController controller, UITableView tableView);
		
		[Export ("searchDisplayController:shouldReloadTableForSearchString:")]
		bool SearchDisplayControllerShouldReloadTableForSearchString (UISearchDisplayController controller, string searchString);
		
		[Export ("searchDisplayController:shouldReloadTableForSearchScope:")]
		bool SearchDisplayControllerShouldReloadTableForSearchScope (UISearchDisplayController controller, int searchOption);
	}
	
	interface UISearchBarDelegateProtocol 
	{
		[Export ("searchBarShouldBeginEditing:")]
		bool SearchBarShouldBeginEditing (UISearchBar searchBar);
		
		[Export ("searchBarTextDidBeginEditing:")]
		void SearchBarTextDidBeginEditing (UISearchBar searchBar);
		
		[Export ("searchBarShouldEndEditing:")]
		bool SearchBarShouldEndEditing (UISearchBar searchBar);
		
		[Export ("searchBarTextDidEndEditing:")]
		void SearchBarTextDidEndEditing (UISearchBar searchBar);
		
		[Export ("searchBar:textDidChange:")]
		void SearchBarTextDidChange (UISearchBar searchBar, string searchText);
		
		[Export ("searchBar:shouldChangeTextInRange:replacementText:")]
		bool SearchBarShouldChangeTextInRangeReplacementText (UISearchBar searchBar, NSRange range, string text);
		
		[Export ("searchBarSearchButtonClicked:")]
		void SearchBarSearchButtonClicked (UISearchBar searchBar);
		
		[Export ("searchBarBookmarkButtonClicked:")]
		void SearchBarBookmarkButtonClicked (UISearchBar searchBar);
		
		[Export ("searchBarCancelButtonClicked:")]
		void SearchBarCancelButtonClicked (UISearchBar searchBar);
		
		[Export ("searchBarResultsListButtonClicked:")]
		void SearchBarResultsListButtonClicked (UISearchBar searchBar);
		
		[Export ("searchBar:selectedScopeButtonIndexDidChange:")]
		void SearchBarSelectedScopeButtonIndexDidChange (UISearchBar searchBar, int selectedScope);
	}

PSPDFCacheDelegate, PSPDFTextSearchDelegate, PSPDFStatusBarStyleHint Inlined correctly.

Having to manually create UISearchDisplayDelegate and UISearchBarDelegate interfaces is really dangerous too because that it can change between iOS versions.

Actually I think I understand why Model was not intended that way, writing override on MonoDevelop and then getting a list of  hundreds of methods to override well that would not be really beautiful.

What about the following proposal from my part Miguel and Rolf, I think its much cleaner and you guys ( I think ) already do that if I understand correctly

Lets take for example the example above 

@interface PSPDFSearchViewController : UITableViewController <UISearchDisplayDelegate, UISearchBarDelegate, PSPDFCacheDelegate, PSPDFTextSearchDelegate, PSPDFStatusBarStyleHint, PSPDFPopoverControllerDismissable>

[BaseType (typeof (UITableViewController))]
interface PSPDFSearchViewController
{
	[Export ("initWithDocument:pdfController:")]
	IntPtr Constructor (PSPDFDocument document, PSPDFViewController pdfController);
		
	[Export ("searchText", ArgumentSemantic.Copy)]
	string SearchText { get; set; }

	// ... And all messages and properties that comes in  PSPDFSearchViewController interface
	
	// Now something like [Adopts/Protocol] Attribute 

	[Adopts ("UISearchDisplayDelegate")]
	UISearchDisplayDelegate SearchDisplayDelegate { get; set; }

	[Adopts ("UISearchBarDelegate")]
	UISearchBarDelegate SearchBarDelegate { get; set; }

	[Adopts ("PSPDFCacheDelegate")]
	PSPDFCacheDelegate CacheDelegate { get; set; }

	[Adopts ("PSPDFTextSearchDelegate")]
	PSPDFTextSearchDelegate TextSearchDelegate { get; set; }

	[Adopts ("PSPDFStatusBarStyleHint")]
	PSPDFStatusBarStyleHint StatusBarStyleHint { get; set; }

	[Adopts ("PSPDFPopoverControllerDismissable")]
	PSPDFPopoverControllerDismissable PopoverControllerDismissable { get; set; }
}

We would have a property for each protocol that an @interface implements if we would like to implement for example SearchDisplayDelegate we would need to create a class that inherits from SearchDisplayDelegate and then override the methods we need on our custom class and then assign it to the corresponding protocol.

I dont know if I explained myself here and If possible

Alex
Comment 5 Peter Steinberger 2013-03-01 05:53:25 UTC
Any updates on this? Took me quite some time that it's a Xamarin bindings bug, and it's a very non-obvious issue where the native part can throw exceptions because a class is overridden in Xamarin. See also https://github.com/Krumelur/MonoTouch-PSPDFKit-bindings/commit/7078f2cb467f3d252a6c5f4d9dcf68417d6fbecf and 5) on this list https://gist.github.com/steipete/5063602.
Comment 6 Alex Soto [MSFT] 2017-02-13 16:17:21 UTC
This isn't reproducible anymore, I think this was fixed with the new [Protocol] support https://developer.xamarin.com/releases/ios/xamarin.ios_7/xamarin.ios_7.0/#Enhanced_protocol_support