Closed Bug 966744 Opened 7 years ago Closed 5 years ago
View Selection Source completely broken
Steps To Reproduce (for example) 1. Open https://developer.mozilla.org/en-US/Add-ons/Code_snippets?redirectlocale=en-US&redirectslug=Code_snippets 2. Select All (Ctrl+A) 3. Right click and Choose "View Selection Source" Actual Results: It shows few lines only Expected Results: It should be 663 lines Regression window(m-i) Good: http://hg.mozilla.org/integration/mozilla-inbound/rev/6fc89dd13c81 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20131219050507 Bad: http://hg.mozilla.org/integration/mozilla-inbound/rev/5de427ba924e Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20131219061007 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=6fc89dd13c81&tochange=5de427ba924e Regressed by: Bug 943519
Alessio, can you have a look at this since it is your patch? Thanks
(In reply to Benjamin Kerensa [:bkerensa] from comment #1) > Alessio, can you have a look at this since it is your patch? Thanks Sure, I'll give it a look ASAP!
Summary: Veiw Selection Source completely broken → View Selection Source completely broken
We should get some tests for this stuff at some point. :(
The regressing bug added rv propagation where there was none before. I bet that View Selection Source relies on failure rv not being propagated somewhere.
Same root cause as bug 964239? Let's see if that one fixed this, too.
This is what happens when running a debug build: WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x804B000A: file /Mozilla-source-local/Mozilla-central/parser/html/nsHtml5TreeOperation.cpp, line 756
By following the precious advices of Benjamin Smedberg, I was able to pinpoint the issue and deliver this fix. I'm working on a mochitest right now to automatically test for this bug.
Assignee: nobody → alessio.placitelli
Attachment #8372779 - Flags: review?(hsivonen)
Just uploaded the patch to the tryserv to be sure: https://tbpl.mozilla.org/?tree=Try&rev=759ff40143c8
The patches for the other bug mentioned in comment 5 landed: https://hg.mozilla.org/mozilla-central/rev/028dbae2c819 This bug is now fixed, right?
We don't need this for branch landing or anything, right?
Henri, AFAIK, the patches from the other bug has not been uplifted to aurora (and the uplift request should probably been done in the other bug)
Comment on attachment 8372779 [details] [diff] [review] bug966744.patch r+ for Aurora only (if approved, requesting approval now.) > [Approval Request Comment] > Bug caused by (feature/regressing bug #): Bug 943519. > User impact if declined: The View Selection Source is broken if the any URL attributes (e.g. href on <a>!) are involved. > Testing completed (on m-c, etc.): An equivalent patch has landed on m-c as part of another bug fix. > Risk to taking this patch (and alternatives if risky): Extremely low-risk. > String or IDL/UUID changes made by this patch: None.
Attachment #8372779 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
In the case of uplift, checkin-needed is not needed. Sheriffs go on the list of approval-mozilla-aurora+ few times a day.
For future reference, it's generally best to leave the aurora approval requests in the original bug rather than moving the patch over - that's just confusing.
Did bug 964239 actually fix this? It seems weird to me to treat that NewURI call as fatal for parsing purposes; it can fail for reasons other than lack of a base URI, no?
(In reply to Boris Zbarsky [:bz] from comment #17) > Did bug 964239 actually fix this? It seems weird to me to treat that NewURI > call as fatal for parsing purposes; it can fail for reasons other than lack > of a base URI, no? If it can fail for other reasons, then it's probably more correct to treat it differently. But, to be honest, I'm not quite an expert so it's better if :hsivonen answers your questions!
Flags: needinfo?(alessio.placitelli) → needinfo?(hsivonen)
Bug 998356 comment 1 seems to have the right reason.
Is this bug still valid?
Assignee: alessio.placitelli → nobody
Hi, I haven't managed to reproduce the issue on the latest Firefox release(45.0.2, Build ID 20160407164938) or the latest Nightly(48.0a1, Build ID 20160415030231) and can confirm this was an issue on the Nightly 29 (20131219061007). Considering this I will mark this issue as Resolved-WORKSFORME. If anyone can still reproduce it, feel free to reopen the issue and provide more information. Thanks, Cipri
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WORKSFORME
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.